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

Adding type hinting #648

Merged
merged 19 commits into from
Jan 6, 2021
Merged

Adding type hinting #648

merged 19 commits into from
Jan 6, 2021

Conversation

shimwell
Copy link
Collaborator

@shimwell shimwell commented Dec 30, 2020

Proposed changes

Adding a little bit of type hinting to a class and a few functions. Just read this guide https://realpython.com/python-type-checking/ and wanted to give it a go.

I noticed the hover-text in VS code picks up these hints however the more interesting thing for me is the consequences this could have for doc strings.

We could in theory define all the argument and return types using type hinting and have this show up in the docs using a sphinx add on https://github.com/agronholm/sphinx-autodoc-typehints

Just making this tiny PR to start the discussion :-)

When building the docs the type now appears in smaller letters and no longer has the optional tag.

before

Screenshot from 2020-12-30 17-21-54

After

Screenshot from 2020-12-30 17-21-42

Types of changes

What types of changes does your code introduce to the Paramak?
Put an x in the boxes that apply

  • Bugfix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Code refactoring
  • Documentation Update (if none of the other choices apply)
  • New tests

Checklist

  • Pep8 applied
  • Unit tests pass locally with my changes
  • I have added tests that prove my fix is effective or that my feature works
  • I have added necessary documentation (if appropriate)

@shimwell shimwell mentioned this pull request Dec 30, 2020
10 tasks
@RemDelaporteMathurin
Copy link

@shimwell nice stuff overall! Just a shame that the "optionnal" tag disappears it was rather useful.

@billingsley-john
Copy link

@shimwell nice stuff overall! Just a shame that the "optionnal" tag disappears it was rather useful.

The optional tag can be added using this type hinting - trying to figure this out now

@billingsley-john
Copy link

@shimwell nice stuff overall! Just a shame that the "optionnal" tag disappears it was rather useful.

The optional tag can be added using this type hinting - trying to figure this out now

from typing import Optional

param: Optional[type]

can be added. Not optimal formatting but gets the job done.

Screenshot from 2021-01-04 09-47-20

@billingsley-john
Copy link

billingsley-john commented Jan 4, 2021

Also, unfortunately, it looks like the suggested way to add multiple type options to an argument is to use Union typing.

e.g.
from typing import Union
param: Union[type1, type2]

But this doesn't look very nice as Union remains as part of the docstring.

For example:

Screenshot from 2021-01-04 10-09-16

@shimwell
Copy link
Collaborator Author

shimwell commented Jan 4, 2021

Also, unfortunately, it looks like the suggested way to add multiple type options to an argument is to use Union typing.

e.g.
from typing import Union
param: Union[type1, type2]

But this doesn't look very nice as Union remains as part of the docstring.

For example:

Screenshot from 2021-01-04 10-09-16

I wonder if readthedocs does this in a nicer way

@shimwell shimwell changed the base branch from develop to develop_combine January 6, 2021 11:47
@shimwell shimwell merged commit 8990ed5 into develop_combine Jan 6, 2021
@billingsley-john billingsley-john deleted the adding_type_hinting branch January 14, 2021 12:32
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.

3 participants