Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove two Pytest warnings due to numpy and pytest mocker in test_dump function #112

Merged
merged 14 commits into from
Oct 25, 2024

Conversation

bobleesj
Copy link
Contributor

@bobleesj bobleesj commented Sep 18, 2024

Closes #133

@bobleesj bobleesj changed the title Remove Two Pytest warnings due to numpy and pytest mocker in test_dump function Remove two Pytest warnings due to numpy and pytest mocker in test_dump function Sep 18, 2024
Copy link
Contributor Author

@bobleesj bobleesj left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ready for review

@bobleesj bobleesj marked this pull request as ready for review September 18, 2024 00:49
Copy link
Contributor

@sbillinge sbillinge left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

please see inline

tests/test_diffraction_objects.py Outdated Show resolved Hide resolved
@bobleesj
Copy link
Contributor Author

Yes, but this is not my point. My point is what behavior are we testing with this test? Not what code are we testing but what behavior?

I hope I get this right - the software behavior is expected to return a Runtime warning message from the input of 1.54. So we capture that warning message? @sbillinge

test.insert_scattering_quantity(
x, y, "q", metadata={"thing1": 1, "thing2": "thing2", "package_info": {"package2": "3.4.5"}}
)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The code now captures the warning:

From:
Screenshot 2024-09-19 at 7 26 55 AM

to:

Screenshot 2024-09-19 at 7 27 35 AM

@sbillinge
Copy link
Contributor

Yes, but this is not my point. My point is what behavior are we testing with this test? Not what code are we testing but what behavior?

I hope I get this right - the software behavior is expected to return a Runtime warning message from the input of 1.54. So we capture that warning message? @sbillinge

I see. You want the code to return a warning if the user inputs a wavelength of 1.54? Why do you want the code to do that? Is 1.54 somehow invalid as a wavelength? Actually, 1.54 is Cu K_alpha wavelength. It is one of the most commonly used x-ray wavelengths in the world. I think it is a perfectly valid wavelength, or am I missing something.

Copy link
Contributor Author

@bobleesj bobleesj left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@sbillinge ready for review - once merged. we can do 3.12 (recut) pypi release

@@ -232,18 +233,22 @@ def test_diffraction_objects_equality(inputs1, inputs2, expected):


def test_dump(tmp_path, mocker):
x, y = np.linspace(0, 10, 11), np.linspace(0, 10, 11)
x, y = np.linspace(0, 5, 6), np.linspace(0, 5, 6)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To recollect our memory: we had this invalid value warning:

RuntimeWarning: invalid value encountered in arcsin
    print(np.rad2deg(2.0 * np.arcsin(q * pre_factor)))

Problem:

In our dummy test_dump function:

The code sets arbitrary q values as:

# x, y = np.linspace(0, 10, 11), np.linspace(0, 10, 11) sets q
# q = np.asarray(q)
# print(q)
[ 0.  1.  2.  3.  4.  5.  6.  7.  8.  9. 10.]

Hence, when we try to convert q to two-theta, using test.wavelength = 1.54:

print(np.rad2deg(2.0 * np.arcsin(q * pre_factor)))
[  0.          14.07850645  28.37532297  43.14126254  58.70709081
  75.57672216  94.66443973 118.15097974 157.27157928          nan
          nan]

We have nan values.

Solution:

Just simply use the valid q range from 0 to 6, etc.

@bobleesj
Copy link
Contributor Author

@sbillinge applied pre-commit - ready for review

@sbillinge sbillinge merged commit 5eedb0d into diffpy:main Oct 25, 2024
3 checks passed
@bobleesj bobleesj deleted the codecov branch October 25, 2024 18:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Remove two pytest warnings for 3.4.3 release
2 participants