Skip to content

Improve type hints #7532

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Open
wants to merge 7 commits into
base: main
Choose a base branch
from
Open

Improve type hints #7532

wants to merge 7 commits into from

Conversation

Armavica
Copy link
Member

@Armavica Armavica commented Oct 10, 2024

Description

Related Issue

  • Closes #
  • Related to #

Checklist

Type of change

  • New feature / enhancement
  • Bug fix
  • Documentation
  • Maintenance
  • Other (please specify):

📚 Documentation preview 📚: https://pymc--7532.org.readthedocs.build/en/7532/

@Armavica Armavica added maintenance no releasenotes Skipped in automatic release notes generation labels Oct 10, 2024
Copy link

codecov bot commented Oct 10, 2024

Codecov Report

Attention: Patch coverage is 96.42857% with 1 line in your changes missing coverage. Please review.

Project coverage is 92.76%. Comparing base (938aff4) to head (b98c47e).
Report is 2 commits behind head on main.

Files with missing lines Patch % Lines
pymc/backends/__init__.py 50.00% 1 Missing ⚠️
Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #7532      +/-   ##
==========================================
+ Coverage   92.69%   92.76%   +0.06%     
==========================================
  Files         104      104              
  Lines       17409    17403       -6     
==========================================
+ Hits        16138    16144       +6     
+ Misses       1271     1259      -12     
Files with missing lines Coverage Δ
pymc/backends/arviz.py 95.76% <ø> (ø)
pymc/backends/base.py 88.44% <ø> (ø)
pymc/distributions/custom.py 95.54% <ø> (ø)
pymc/distributions/distribution.py 91.89% <100.00%> (ø)
pymc/gp/hsgp_approx.py 88.39% <ø> (ø)
pymc/initial_point.py 99.09% <100.00%> (+0.05%) ⬆️
pymc/logprob/mixture.py 95.70% <100.00%> (ø)
pymc/model/fgraph.py 97.94% <ø> (ø)
pymc/sampling/forward.py 96.15% <ø> (ø)
pymc/sampling/jax.py 94.78% <ø> (ø)
... and 3 more

Comment on lines 604 to +605
mu: np.ndarray | float,
rng: Optional[np.random.Generator] = None,
size: Optional[Tuple[int]] = None,
rng: np.random.Generator | None = None,
Copy link
Member

Choose a reason for hiding this comment

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

For the example: mu is always np.ndarray, rng is always provided never None

f"Number of initval dicts ({len(overrides)}) does not match the number of chains ({chains})."
return ipfns

assert isinstance(overrides, Sequence) and len(overrides) == chains
Copy link
Member

Choose a reason for hiding this comment

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

Already checked above

Copy link
Member Author

Choose a reason for hiding this comment

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

It's also related to the meaning of assert. I see it less as a control flow, more as an executable comment. Converting this to a comment would leave the possibility that the comment goes out of date.
Is this function typically hot?
Also, asserts are disabled by python -O, perhaps this is something that we could do?

Copy link
Member

Choose a reason for hiding this comment

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

I don't think anyone ever disables asserts tbh

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
maintenance no releasenotes Skipped in automatic release notes generation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants