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

Use pytest for test_units #681

Merged
merged 3 commits into from
Mar 7, 2025
Merged

Conversation

oerc0122
Copy link
Collaborator

Description of work

  • Refactor units testing to use pytest
  • Refactor _Unit class to use dunders
  • Replace Equivalences dict with defaultdict

Fixes
N/A

To test
Standard tests

Refactor _Unit class to use dunders better
@oerc0122 oerc0122 added the CI/CD Something concerning the CI/CD pipeline label Feb 26, 2025
@oerc0122 oerc0122 self-assigned this Feb 26, 2025
"""Ceil of a _Unit value in canonical units.

>>> print(measure(10.2, 'm/s').ceiling())
10.0000 m / s
Copy link
Collaborator

Choose a reason for hiding this comment

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

I know that these docstrings were already written like this before you made changes, but I think that the examples given here are wrong.

The docstrings below for __floor__ and __round__ should be checked too.

Overall, I would expect 3.6 m/s to be 12.96 km/h, and the ceil, round, floor to give 13, 13 and 12, respectively.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Reworked the docstrings throughout and made sure it was valid under doctest.

Copy link
Collaborator

@MBartkowiakSTFC MBartkowiakSTFC left a comment

Choose a reason for hiding this comment

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

I have not found any problems. Looks good.

@oerc0122 oerc0122 merged commit 712fbbc into ISISNeutronMuon:protos Mar 7, 2025
34 checks passed
@oerc0122 oerc0122 deleted the pytestise-units branch March 7, 2025 11:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CI/CD Something concerning the CI/CD pipeline
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants