test function on_xtype #192
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #192 +/- ##
==========================================
- Coverage 96.53% 96.27% -0.27%
==========================================
Files 8 8
Lines 289 322 +33
==========================================
+ Hits 279 310 +31
- Misses 10 12 +2
|
|
I am a little unclear what this is doing. Could you maybe hit down some words to explain the high level view? |
I think this is for checking if |
Is this a new function? One that can be used instead of |
It's an old function. This PR is to check that it returns the right array and raise an error if it's an invalid xtype. It doesn't replace on_tth but simply calls on_tth if xtype is tth. |
sbillinge
left a comment
sbillinge
left a comment
There was a problem hiding this comment.
pls see inline comment
tests/test_diffraction_objects.py
Outdated
| @pytest.mark.parametrize("inputs, expected", params_on_xtype) | ||
| def test_on_xtype(inputs, expected): | ||
| test = DiffractionObject() | ||
| test.on_tth = np.array([inputs[1], inputs[0]]) |
There was a problem hiding this comment.
This is not really how DO's should be used. The tth, q and d arrays are somehow all different versions of the same array so we shouldn't be able to set them independently like this. If we can now this is not desirable behavior, so we probably don't want a test that does this. We should probably create a set property operator that generates all the arrays whenever a single one is specified in this way. But for sure, we don't want a test with this undesirable behavior....
|
@sbillinge ready for review! |
sbillinge
left a comment
sbillinge
left a comment
There was a problem hiding this comment.
please see inline.. We can discuss, but this should behave the same as DO.on_q() etc., and I think they should maybe return list of 1D arrays.
|
|
||
| from diffpy.utils.tools import get_package_info | ||
| from diffpy.utils.transforms import d_to_q, d_to_tth, q_to_d, q_to_tth, tth_to_d, tth_to_q | ||
|
|
There was a problem hiding this comment.
I think we want to return a list of two 1D arrays
| return ( | ||
| f"WARNING: I don't know how to handle the xtype, '{xtype}'. Please rerun specifying an " | ||
| f"xtype from {*XQUANTITIES, }" | ||
| ) |
sbillinge
left a comment
sbillinge
left a comment
There was a problem hiding this comment.
one small tweak inline
tests/test_diffraction_objects.py
Outdated
| with pytest.raises( | ||
| ValueError, | ||
| match=re.escape( | ||
| f"WARNING: I don't know how to handle the xtype, 'invalid'. Please rerun specifying an " |
There was a problem hiding this comment.
I think are now raising an exception, right? So this isn't a warning. Just remove WARNING: Python will handle the rest.
2 participants
issue #186: test function for
on_xtype