-
Notifications
You must be signed in to change notification settings - Fork 52
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
Add type hinting #193
Comments
linking: openjournals/joss-reviews#3285 |
@grburgess: If you don't mind, I propose splitting this into two issues: (1) Fill in some missing docstrings and (2) type annotations. I would argue that type hinting is out of scope for now since the underlying libraries (numpy and theano/aesara) only have very limited support for typing at the moment (e.g. numpy/numpy#7370 & pymc-devs/pytensor#200). Since these are the main return types of all of the |
I will split it up. |
@dfm I agree that for the time being it is only icing on the cake at the moment and can be ignored for the review. The real power for now is when using editors that support LSP. The code completion and code hinting become a very powerful tool for real time code and syntax checking. |
Great - yes, it really is awesome for code completion, etc. I have a few other projects (that don't depend on numpy) where I've loved using Python typing. I'm going to remove the |
I can also take a stab at putting some hinting in as PRs. |
That would be very awesome if you're keen, but no pressure! |
I would suggest (though it is again stylist and not required) to start adding type hints as it is being moved into the standard python world. This helps many code editors with completion and allows for robust tests using type checkers such as
mypy
.Though this is not my field, I can see this code as being a framework where other researchers import it and build future models, thus type hinting would keep them from stumbling over the various inputs and outputs related to
theano
andpymc3
which are vital to the functionality of the code. I note that neither of these frameworks are typed, but the size ofexoplanet
is manageable enough (with lots of contributors) to slowly add this in.The text was updated successfully, but these errors were encountered: