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

Make validators compatible with Quantity type annotations that preserve units #1414

Closed
namurphy opened this issue Jan 31, 2022 · 3 comments · May be fixed by #1661
Closed

Make validators compatible with Quantity type annotations that preserve units #1414

namurphy opened this issue Jan 31, 2022 · 3 comments · May be fixed by #1661
Labels
effort: low Requiring perhaps a few hours, but less than a day feature request Issues requesting a new feature or enhancement

Comments

@namurphy
Copy link
Member

In #1413, I raised an issue to adopt astropy.units.Quantity Quantity type annotations that preserve units.

Before we can resolve #1413, we would need to make the decorators in plasmapy.utils.decorators.validators compatible with annotations like:

import astropy.units as u
from typing import Optional, Union

def f(x: u.Optional[u.Quantity[u.m]]) -> u.Quantity[u.m]):
    ...

def g(x: Union[u.Quantity[u.K], u.Quantity[u.eV]]) -> Optional[u.Quantity[u.K]]:
    ...

This might be a good first issue for someone who is familiar with Python but not PlasmaPy.

@namurphy namurphy added the effort: low Requiring perhaps a few hours, but less than a day label Jan 31, 2022
@RAJAGOPALAN-GANGADHARAN
Copy link
Contributor

@namurphy Can I give this a shot?

@rocco8773
Copy link
Member

@namurphy and I have gone back and forth about this implementation. I honestly don't like Quantity's type annotation in its current form because it will make the signature line in our code and in our docs really messy. With that said, I'm still up for supporting it, but only if we still support the older style of typing. This issue would be additional functionality and not replacement functionality.

To implement this, you are actually going to have to modify CheckUnits instead of ValidateQuantities, since ValidateQuantities is a subclass of CheckUnits. I think you'll only need to touch these lines of code.

As a side note, @namurphy maybe we should start looking into implementing Python stub files (.pyi). I don't know their workings in detail but it might be worth exploring.

@RAJAGOPALAN-GANGADHARAN With all my rambling over, yes have at it!!!

@github-actions github-actions bot added the Stale Dormant issues & PRs which will be automatically closed if the label is not removed. label May 15, 2023
@namurphy namurphy removed the Stale Dormant issues & PRs which will be automatically closed if the label is not removed. label May 23, 2023
@namurphy namurphy added the feature request Issues requesting a new feature or enhancement label Aug 30, 2023
@namurphy
Copy link
Member Author

Closed by #2346.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
effort: low Requiring perhaps a few hours, but less than a day feature request Issues requesting a new feature or enhancement
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants