Skip to content

Conversation

@lucianopaz
Copy link
Member

@lucianopaz lucianopaz commented Sep 27, 2022

What is this PR about?

Update mypy version to 0.981. According to this, it should fix the failed CI encountered in both #6151 and #6147

Checklist

Major / Breaking Changes

  • ...

Bugfixes / New features

  • ...

Docs / Maintenance

  • ...

@lucianopaz lucianopaz added pre-commit no releasenotes Skipped in automatic release notes generation labels Sep 27, 2022
@lucianopaz lucianopaz requested a review from maresb September 27, 2022 15:04
@maresb
Copy link
Contributor

maresb commented Sep 27, 2022

Is the other mypy warning new as of 0.981?

Perhaps the return type should be Any instead of T?

@maresb
Copy link
Contributor

maresb commented Sep 27, 2022

Ah, or should DictToArrayBijection be a generic on T?

EDIT: I got confused here, this should instead refer to Compose.

@lucianopaz
Copy link
Member Author

Looks like returning Any did the trick.

I ran pre-commit on all the files and that was the only new warning that popped up.

Copy link
Contributor

@maresb maresb left a comment

Choose a reason for hiding this comment

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

I'm thinking that Compose should be a generic

pymc/blocking.py Outdated
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
def __call__(self, x: RaveledVars) -> Any:
def __call__(self, x: RaveledVars) -> T:

Copy link
Contributor

Choose a reason for hiding this comment

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

I think you should change class Compose: above to class Compose(Generic[T]):. This way, the T variable from __init__ and __call__ will be linked, and the type variable will actually define a relation.

@lucianopaz
Copy link
Member Author

I'm thinking that Compose should be a generic

Thanks @maresb! The Generic[T] class annotation fixed the type hints

@codecov
Copy link

codecov bot commented Sep 27, 2022

Codecov Report

Merging #6154 (c44e0e5) into main (1417785) will increase coverage by 1.07%.
The diff coverage is 100.00%.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #6154      +/-   ##
==========================================
+ Coverage   91.93%   93.00%   +1.07%     
==========================================
  Files         101       91      -10     
  Lines       20948    20707     -241     
==========================================
+ Hits        19258    19259       +1     
+ Misses       1690     1448     -242     
Impacted Files Coverage Δ
pymc/aesaraf.py 93.38% <ø> (ø)
pymc/distributions/transforms.py 100.00% <ø> (ø)
pymc/model.py 88.23% <ø> (ø)
pymc/blocking.py 95.12% <100.00%> (ø)
pymc/distributions/continuous.py 97.50% <100.00%> (ø)
pymc/distributions/multivariate.py 92.31% <100.00%> (ø)
pymc/smc/smc.py 97.24% <100.00%> (ø)
pymc/variational/updates.py 92.11% <100.00%> (ø)
pymc/parallel_sampling.py 85.80% <0.00%> (-1.00%) ⬇️
pymc/distributions/__init__.py
... and 10 more

@lucianopaz lucianopaz requested a review from twiecki September 27, 2022 16:17
@ricardoV94 ricardoV94 merged commit b12c189 into pymc-devs:main Sep 27, 2022
@lucianopaz lucianopaz deleted the mypy_upgrade branch September 27, 2022 16:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

no releasenotes Skipped in automatic release notes generation pre-commit

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants