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

Expose SS Typehints #468

Merged
merged 3 commits into from
Jan 31, 2024
Merged

Conversation

MattToast
Copy link
Member

Add and ship py.typed marker per pep-561. This change will help change the mypy output for this script

from smartsim import Experiment
from smartredis import Client

exp = Experiment(b"my-exp")  # Should err expected str
client = Client(False)

reveal_type(exp)
reveal_type(client)

from this

$ mypy script.py
script.py:1: error: Skipping analyzing "smartsim": module is installed, but missing library stubs or py.typed marker  [import-untyped]
script.py:1: note: See https://mypy.readthedocs.io/en/stable/running_mypy.html#missing-imports
script.py:2: error: Skipping analyzing "smartredis": module is installed, but missing library stubs or py.typed marker  [import-untyped]
script.py:7: note: Revealed type is "Any"
script.py:8: note: Revealed type is "Any"
Found 2 errors in 1 file (checked 1 source file)

to this

$ mypy script.py
script.py:4: error: Argument 1 to "Experiment" has incompatible type "bytes"; expected "str"  [arg-type]
script.py:7: note: Revealed type is "smartsim.experiment.Experiment"
script.py:8: note: Revealed type is "smartredis.client.Client"
Found 1 error in 1 file (checked 1 source file)

@MattToast MattToast added area: api Issues related to API changes type: usability Issues related to ease of use labels Jan 29, 2024
@MattToast MattToast self-assigned this Jan 29, 2024
Copy link
Collaborator

@al-rigazzi al-rigazzi left a comment

Choose a reason for hiding this comment

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

LGTM! Thanks for taking another step towards modern Python!

Copy link

codecov bot commented Jan 29, 2024

Codecov Report

Attention: 4 lines in your changes are missing coverage. Please review.

Comparison is base (50aa382) 90.80% compared to head (8de5f21) 90.43%.
Report is 2 commits behind head on develop.

Additional details and impacted files

Impacted file tree graph

@@             Coverage Diff             @@
##           develop     #468      +/-   ##
===========================================
- Coverage    90.80%   90.43%   -0.38%     
===========================================
  Files           60       60              
  Lines         3829     3837       +8     
===========================================
- Hits          3477     3470       -7     
- Misses         352      367      +15     
Files Coverage Δ
smartsim/database/orchestrator.py 86.85% <100.00%> (ø)
smartsim/entity/dbobject.py 64.81% <100.00%> (-0.33%) ⬇️
smartsim/entity/ensemble.py 97.61% <ø> (-1.59%) ⬇️
smartsim/entity/model.py 95.51% <100.00%> (+0.08%) ⬆️
smartsim/ml/data.py 94.37% <75.00%> (ø)
smartsim/ml/tf/data.py 88.63% <66.66%> (ø)
smartsim/_core/utils/redis.py 80.39% <50.00%> (-2.59%) ⬇️

... and 6 files with indirect coverage changes

Copy link
Collaborator

@al-rigazzi al-rigazzi left a comment

Choose a reason for hiding this comment

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

Re-approving and praising the effort of catching all those type errors!

@MattToast MattToast merged commit b160c05 into CrayLabs:develop Jan 31, 2024
34 checks passed
@MattToast MattToast deleted the expose-type-hints branch February 16, 2024 02:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: api Issues related to API changes type: usability Issues related to ease of use
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants