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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add missing type hint imports, as suggested by mypy. #1397

Merged

Conversation

obi1kenobi
Copy link
Contributor

A few minor fixes for type hint issues reported by mypy:

arviz/stats/stats.py:1133: error: Name 'Dict' is not defined
arviz/stats/stats.py:1133: note: Did you forget to import it from "typing"? (Suggestion: "from typing import Dict")
arviz/data/io_pyro.py:21: error: Name 'Optional' is not defined
arviz/data/io_pyro.py:21: note: Did you forget to import it from "typing"? (Suggestion: "from typing import Optional")
arviz/data/io_numpyro.py:19: error: Name 'Optional' is not defined
arviz/data/io_numpyro.py:19: note: Did you forget to import it from "typing"? (Suggestion: "from typing import Optional")

I also replaced use of builtins.callable with typing.Callable in type hints since mypy doesn't consider the lowercase version a valid type:

arviz/data/io_pyro.py:22: error: Function "builtins.callable" is not valid as a type
arviz/data/io_pyro.py:22: note: Perhaps you need "Callable[...]" or a callback protocol?

Unfortunately, this isn't a comprehensive set of type hint fixes -- mypy is still quite unhappy even with these changes applied. I just figured something is better than nothing 馃槂

@codecov
Copy link

codecov bot commented Sep 27, 2020

Codecov Report

Merging #1397 into master will increase coverage by 0.00%.
The diff coverage is 100.00%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #1397   +/-   ##
=======================================
  Coverage   91.63%   91.63%           
=======================================
  Files         105      105           
  Lines       10965    10967    +2     
=======================================
+ Hits        10048    10050    +2     
  Misses        917      917           
Impacted Files Coverage 螖
arviz/data/io_numpyro.py 95.27% <100.00%> (+0.03%) 猬嗭笍
arviz/data/io_pyro.py 97.35% <100.00%> (+0.01%) 猬嗭笍
arviz/stats/stats.py 96.31% <100.00%> (酶)

Continue to review full report at Codecov.

Legend - Click here to learn more
螖 = absolute <relative> (impact), 酶 = not affected, ? = missing data
Powered by Codecov. Last update 24ce65a...569d052. Read the comment docs.

@ahartikainen
Copy link
Contributor

We currently don't implement or require type hints, but I think it could be a good addition to our lib. Is there yet a good solution for numpy / xarray related problems (e.g. we would not need to use Any)

@ahartikainen ahartikainen merged commit d6794f5 into arviz-devs:master Sep 27, 2020
@obi1kenobi obi1kenobi deleted the fix_missing_type_hint_imports branch September 27, 2020 18:21
@obi1kenobi
Copy link
Contributor Author

Is there yet a good solution for numpy / xarray related problems (e.g. we would not need to use Any)

It seems like some solid progress is being made within the packages themselves: numpy/numpy#7370 It doesn't seem like it's super thorough yet, but it seems quite actively being worked on so it'll hopefully get even better quickly.

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.

None yet

3 participants