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

(internal) Typing support #244

Merged
merged 25 commits into from
May 21, 2020
Merged

(internal) Typing support #244

merged 25 commits into from
May 21, 2020

Conversation

henryiii
Copy link
Member

@henryiii henryiii commented May 20, 2020

Adding typing support. Using Python 2-compatible type comments.

Currently we just have the minimum + one file fully done (pidid/functions), and basic support on Particle and with some of the other functions.

MyPy has caught a variety of potential crashes related to multiplying values with None, abs(None), etc. Whenever you return None as part of your API, that means it has to be checked at every step down the line...

We also now have full, type-checked support for using any object that SupportsInt (that is, has a __int__() special method), so you can now pass Particles directly to any of these functions!

We need to rethink or replace our loops over functions - PDGID is the main suspect. There's several options: Maybe we can simply write name = _functions.name and that will work (might be a problem in Python 2). Maybe we can simply remove the duplication of functions and methods, and make sure they work as free functions too, that is, PDGID.name(x) at a replacement for PDGID(x).name ; that would only work on Python 3, however; 2 users would need to make the object to use the method. Finally, we could just duplicate this and say that this is mostly static, so having things in two places is not too bad (the computations would be in once place, just the wrappers would be duplicated).

This fixes the ZipApp (which now requires Python 3.5+, is a bit smaller, and has a simpler setup). Dropped usage of file input, which was really designed for changing files listed on the command line and didn't work with file objects. Added a somewhat internal "_name" parameter that will overwrite the recorded file name (since some file objects do not have names, like files in zip directories).

Fixes #245. Closes #237.

To be clear: This targets 0.10. External typing support (for dependent libraries) will go into a later version as what we have is a bit hacky, but it still fixed some possible bugs and helped in a speed up the code by a factor of 100, which isn't bad.

@henryiii henryiii marked this pull request as ready for review May 20, 2020 22:15
@henryiii henryiii changed the title Typing support (internal) Typing support May 21, 2020
@henryiii henryiii force-pushed the henryiii/mypy branch 2 times, most recently from 92093d0 to e72997d Compare May 21, 2020 00:23
@henryiii
Copy link
Member Author

I think this is the last item before release.

Copy link
Member

@eduardo-rodrigues eduardo-rodrigues left a comment

Choose a reason for hiding this comment

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

A lot of great improvements! Really nice.

@eduardo-rodrigues
Copy link
Member

eduardo-rodrigues commented May 21, 2020

I think this is the last item before release.

You did quite a lot for the new release in the end! Honestly, I would have added the typing support after the release … but now that's done :-).

@eduardo-rodrigues
Copy link
Member

We also now have full, type-checked support for using any object that SupportsInt (that is, has a int() special method), so you can now pass Particles directly to any of these functions!

Oh, this then needs to be documented somewhere, probably in the README at least, with some example?

We need to rethink or replace our loops over functions - PDGID is the main suspect. There's several options: Maybe we can simply write name = _functions.name and that will work (might be a problem in Python 2). Maybe we can simply remove the duplication of functions and methods, and make sure they work as free functions too, that is, PDGID.name(x) at a replacement for PDGID(x).name ; that would only work on Python 3, however; 2 users would need to make the object to use the method. Finally, we could just duplicate this and say that this is mostly static, so having things in two places is not too bad (the computations would be in once place, just the wrappers would be duplicated).

I'm not sure I understand this. For sure it makes sense to have the free functions and the identical methods in PDGID.

@henryiii
Copy link
Member Author

henryiii commented May 21, 2020

Example of what could be done eventually:

>>> from particle import PDGID
>>>
>>> pid = PDGID(211)
>>> pid
<PDGID: 211>
>>> pid.is_meson
True
>>> pid = PDGID(99999999)
>>> pid
<PDGID: 99999999 (is_valid==False)>

For convenience, all properties of the PDGID class work as standalone functions in Python 3:

>>> PDGID.is_meson(211)
True

@henryiii henryiii merged commit 0abf3b2 into master May 21, 2020
@henryiii henryiii deleted the henryiii/mypy branch May 21, 2020 13:56
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.

Import time ZipApp fixes
2 participants