Add mypy to CI job and type hints for one class.

[eliotwrobson]

Changes to test type hints and integrating a typechecker into the CI job.
This was to test feasibility of adding type annotations to the package (per
the issue). This was surprisingly easy to do, I basically just copied the types
listed in the docstring.

Relevant issues/PRs:

#330

Squash and Merge summary

Add type hints to pooch/core.py and create a new typing submodule for custom type classes, and add it to the API Reference. Run mypy on CI to perform type checks, and create new targets in the Makefile. Extend the list of dependencies required to run the type checks.

[welcome]

💖 Thank you for opening your first pull request in this repository! 💖

A few things to keep in mind:

• Authorship Guidelines: Our rules for giving you credit for your contributions, including authorship on publications and Zenodo archives.
• Code of Conduct: How we expect people to interact in our projects.

⭐ No matter what, we are really grateful that you put in the effort to do this! ⭐

[eliotwrobson]

Hi @santisoler, is there anything I could do to help move this along? This change would be helpful to me when working with this library, and it seems like there has been interest from others as well.

[santisoler]

Hi @eliotwrobson. Yes, I totally agree. Type hints is something people would like to see in Pooch (me included!).

I left a few comments here so we can move forward with this PR.

Importing from typing

I noticed that in this PR you import typing as t, and then use each one of the types in signatures with t.<TYPE>, like t.Optional, t.List`, etc.

Why not importing the required types directly? Like from typing import .... Most of the codebases that use type hints use the modules like this:

from typing import Optional

def f(x: Optional[float] = None):
    ...

I think having that extra t. creates some clutter in the signatures, specially when we need to use several types. What do you think?

Do we need typing_extensions?

I noticed that you added typing_extensions as a dependency. I'm curious if this strictly necessary. I left a comment below where we can continue the conversation.

Minor things to do

Before I forget, we need to add the new style requirements to environment.yml as well. Thanks for adding them to requirements-style.txt and to add a new target to the Makefile!

---

Thanks again for the effort you put in this! I really appreciate that we can move forward with type hints in Pooch!

[eliotwrobson]

@santisoler I've responded to your comments. I've added mypy to the environment.yml, removed typing_extensions, and changed the typing import. Also, let me know if you want any help with #453, since I'm currently using ruff to autoformat everything locally.

[santisoler]

Hi @eliotwrobson! Thanks for addressing my comments, I appreciate it.

I've just pushed a few changes to your branch, hope you don't mind (feel free to revert changes if you disagree with them). I managed to move the new type classes to their own pooch.typing module. I solved the circular loop by conditionally importing Pooch only if typing.TYPE_CHECKING is True. I took the liberty to rename some of the names to reflect better their meaning. For example, Actions to Action, since the type represents a single str that matches a single action. I also renamed FilePath for PathType, since this type hint is usually used for directories, and not only files. I also added the new submodule to the API reference in the docs. Please, let me know what do you think about these changes.

[eliotwrobson]

I've just pushed a few changes to your branch, hope you don't mind (feel free to revert changes if you disagree with them).

No worries, thank you for making these changes! Everything looks good to me. Would you like to add typing to everything in the library before merging this? Or merge as-is and incrementally add typing over a number of PRs? This currently doesn't have the py.typed file, so client code won't be able to see the annotations that have been added so far, which IMO is good before everything in the library has had types added.

[santisoler]

I think it would be better to merge this PR and work on adding other type hints incrementally in other PRs. That would things easier for reviewing, for avoiding conflicts and to also see some progress.

I'm afraid I'm not familiar with the py.typed file, so if you think it's worth adding it while we work on it (instead of waiting for having everything type hinted), feel free to do so. I'll try to get more familiar with it in the meantime.

[eliotwrobson]

@santisoler sounds good. I don't think it's worth adding py.typed until everything has been type annotated, since the presence of this file exposes the annotations to client code, and I don't think this is a good idea until annotations are on everything (which shouldn't take too long since this library isn't too huge).

Let me know if you'd like more changes here before merging. If not, I think everything looks good, so after this is merged I can get started on annotations in other files.

[santisoler]

I don't think it's worth adding py.typed until everything has been type annotated, since the presence of this file exposes the annotations to client code, and I don't think this is a good idea until annotations are on everything (which shouldn't take too long since this library isn't too huge).

I see. Sorry, I misunderstood your previous comment. Let's forget about the py.typed file for now then.

Let me know if you'd like more changes here before merging. If not, I think everything looks good, so after this is merged I can get started on annotations in other files.

I think we are good to go. I just made a small change to doc/conf.py: the typing.List is deprecated, so I replaced it for the builtin list type.

I'm merging this! 🚀 Thanks again for pushing forward with this and for the hard work.

Feel free to open another PR for type hinting the rest of Pooch whenever you want, no rush!. And also, feel free to split the work in several PRs.