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

Replace check_quantity with `validate_quantities #722

Merged
merged 255 commits into from
Dec 4, 2019

Conversation

rocco8773
Copy link
Member

Replace the used of the decorate check_quantity with validate_quantities and, then, permanently remove check_quantity from the code base.

Notes

rocco8773 added 30 commits June 19, 2019 16:32
…r all methods and class; update typing annotations; define property `checks`
…od instance)...this improves readability of documentation without hindering functionality
…ues`, `test_cv_method__check_value`, and `test_cv_preserves_signature`; define a `foo` function to be wrapped (bound to `TestCheckValues`
…fine more parameters that defined by function; move this PlasmaPyWarning from CheckValues.__call__ to CheckValues._get_checks
…() parameter is not in BoundArguments.arguments since BoundArguments.apply_defaults() is called ahead of time and covers this case
… class `CheckValues` called as a decorator
… attribute `_check_defaults` (being more pythonic!); updated tests accordingly
…`equivalencies`, add comments to `_default_checks`, fix improper indent when defining/conditioning 'equivalencies'
@rocco8773
Copy link
Member Author

I think it would be best to block out the chemical_potential code and open an issue.

Upon a little more investigation, there seems to be a semi-systematic error in the way astropy converts between SI prefix units when the units have powers. --scratches head--

image

If you notice, the error is always a multiple of of 2 ** 17 = 131072. This makes me think the error is originating from how python and/or NumPy is doing the calculation. Maybe some kind of bit operation that is going awry.

Of course, in the example above the error could be in either the subtraction or the unit conversion, or both.

@StanczakDominik
Copy link
Member

https://mail.python.org/pipermail/scipy-dev/2019-December/023881.html this thread probably isn't relevant, but the Python implementation of pow was mentioned, and it felt like there may be a connection.

@StanczakDominik
Copy link
Member

The 2 ** 17 thing looks like we'll indeed need to report it upstream to astropy... I have no idea what to do with it otherwise.

@rocco8773 rocco8773 marked this pull request as ready for review December 3, 2019 19:22
rocco8773 added a commit that referenced this pull request Dec 3, 2019
implementation of `validate_quantities` is in PR #722
# Conflicts:
#	plasmapy/formulary/tests/test_transport.py
#	plasmapy/utils/decorators/__init__.py
#	plasmapy/utils/decorators/checks.py
#	plasmapy/utils/decorators/tests/test_checks.py
#	plasmapy/utils/decorators/tests/test_validators.py
#	plasmapy/utils/decorators/validators.py
… referencing bug outlined in issue PlasmaPy#726...add raising of NotImplementedError
…Error is made redundant by decorator `validate_quantities()`
@rocco8773 rocco8773 merged commit 9ba3f4e into PlasmaPy:master Dec 4, 2019
@rocco8773 rocco8773 deleted the implement_validators branch October 23, 2020 19:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
effort: medium Requiring perhaps ∼1–3 days plasmapy.formulary Related to the plasmapy.formulary subpackage priority: medium Issues & PRs of moderate urgency and importance refactoring ♻️ Improving an implementation without adding new functionality
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants