Add is_polar() utility for classifying polar space groups
#176
Conversation
for more information, see https://pre-commit.ci
There was a problem hiding this comment.
I think this looks very good. I don't see anything objectionable except that I found the test really hard to understand. I think that is on me not understanding pytest that well, but maybe you can spruce up the docstring to help others understand it in the future.
|
|
||
|
|
||
| @pytest.fixture(params=sgtbx_polar_classification()) | ||
| def polar_by_xhm(request): |
There was a problem hiding this comment.
admittedly, i do not understand fixtures all that well, but i found this file surprisingly difficult to read. i think this might be helped by adding
Parameters
---------------
request : ??????
to the docstring.
There was a problem hiding this comment.
technically speaking, the request is of type _pytest.fixture.SubRequest. I'll update this docstring to be more descriptive of what is going on rather than explicit about the types of things. The gist of this pair of functions is that sgtbx_polar_classification() yields an iterable. The polar_by_xhm fixture then packages that in a more convenient form such that the test function is provided a tuple of (str, bool) that is parametrized over all entries in the iterable.
This recipe for building pytest fixtures is pretty prevalent in our conftest.py files.
Codecov Report
@@ Coverage Diff @@
## main #176 +/- ##
=======================================
Coverage 98.31% 98.31%
=======================================
Files 44 44
Lines 1718 1724 +6
=======================================
+ Hits 1689 1695 +6
Misses 29 29
Flags with carried forward coverage won't be shown. Click here to find out more.
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. |
Aligning maps of isomorphous structures can benefit from classifying polar space groups. This is useful for distinguishing which cases have a discrete set of alternative origins vs. continuous alternatives (see #174).
This PR implements a
reciprocalspaceship.utils.is_polar()utility function that classifies a space group as polar. This function is tested against the 530 spacegroup settings insgtbx.