1) Capture no wavelength UserWarning for test_q_to_thh method 2) discuss passing variables to @pytest.mark.parametrize
#225
Conversation
|
Warning! No news item is found for this PR. If this is a user-facing change/feature/fix, |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #225 +/- ##
===========================================
+ Coverage 99.70% 100.00% +0.29%
===========================================
Files 8 8
Lines 340 343 +3
===========================================
+ Hits 339 343 +4
+ Misses 1 0 -1
|
tests/test_transforms.py
Outdated
| def test_q_to_tth(wavelength, q, expected_tth): | ||
|
|
||
| if wavelength is None: | ||
| with pytest.warns(UserWarning, match="INFO: no wavelength has been specified"): |
There was a problem hiding this comment.
Second, here, it feels counter productive to write all the warning msgs:
UserWarning: INFO: no wavelength has been specified. You can continue to use the DiffractionObject but some of its powerful features will not be available. To specify a wavelength, set diffraction_object.wavelength = [number], where diffraction_object is the variable name of you Diffraction Object, and number is the wavelength in angstroms.
assuming we have a specific test func for this warning msg.
Please suggest if this is okay @sbillinge
There was a problem hiding this comment.
I like the user warning written out because the test defines the behavior. So the workflow is to write the test and review it until we like it. Then write the code. Here, it slipped by but we don't want "INFO" here. It is either a WARNING or an INFO, it can't be both. Also, I think when we are describing code it is better to write the code than describe it in words, so say ".... To speciffy a wavelength, if my_object = DiffractionObject(xarray, yarray, "tth") then you can type my_object.wavelength = 1.54` if the wavelength is 1.54 angstroms." Something like that.
If test the same message in more than one test we can assign it to a variable. But it is better if it is closer to the test and we are not importing it orsomething.
|
@sbillinge ready for review - cc'ing @yucongalicechen for our discussions moving forward. |
There was a problem hiding this comment.
@sbillinge ready for review - there are still warnings but prob it would be best to create a couple of PRs by theme.
bobleesj
changed the title
UserWarning for test_q_to_thh methodUserWarning for test_q_to_thh method 2) discuss passing variables to @pytest.mark.parametrize
There was a problem hiding this comment.
These improvements are excellent! Thanks @bobleesj ! I left a couple of small tweaks and comments on your suggestions to make it even better. I think done of these lessons can carry over to better group standards in general so it is not wasted effort to get this right 👍
src/diffpy/utils/transforms.py
Outdated
| "No wavelength has been specified. You can continue to use the DiffractionObject, but " | ||
| "some of its powerful features will not be available. " | ||
| "To specify a wavelength, if you have do = DiffractionObject(xarray, yarray, 'tth'), " | ||
| "you may set do.wavelength = 1.54 with the unit in angstroms." |
There was a problem hiding this comment.
Maybe it is clearer to say than 1.54 here, it add "for a wavelength of 1.54 angstroms". Which would someone like Fillipo find clearer?
tests/test_transforms.py
Outdated
| "No wavelength has been specified. You can continue to use the DiffractionObject, but " | ||
| "some of its powerful features will not be available. " | ||
| "To specify a wavelength, if you have do = DiffractionObject(xarray, yarray, 'tth'), " | ||
| "you may set do.wavelength = 1.54 with the unit in angstroms." |
There was a problem hiding this comment.
Please see the small tweak above
Yes, I am recording these lessons. I will sync with the GitLab page: https://gitlab.thebillingegroup.com/resources/group-wiki/-/wikis/Lessons-learned-in-Billinge-Group |
There was a problem hiding this comment.
@sbillinge ready for review - the only modification was done on wavelength_warning_emsg as shown in the latest commit.
Also created an issue #229 for refactoring test_on_xtype
Here, the goal is to discuss how to write good tests using
@pytest.mark.parametrizeand also test warning messages.