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

Fix type hints #1581

Open
OriolAbril opened this issue Feb 24, 2021 · 7 comments
Open

Fix type hints #1581

OriolAbril opened this issue Feb 24, 2021 · 7 comments

Comments

@OriolAbril
Copy link
Member

OriolAbril commented Feb 24, 2021

The latest mypy version now checks more files, see #1528 (comment). The errors listed in the comment should be fixed. The couple comments after the linked one explain how to do so.

EDIT: The mypy errors encountered while using typing copilot are only the tip of the iceberg, we should check the current type hints and make sure that inputs are only type hinted as tuple or lists when they must be of that type and use sequence or iterator otherwise.

Note: the priority of this is low, the code works perfectly fine, this only affects users of mypy on arviz that will get error false positives, mypy will complain about the types when the code has no issues.

@utkarsh-maheshwari
Copy link
Contributor

I read the discussion.
So basically, what I understood is instead of passing var_names as a tuple in the functions, we need to convert to sequence data type.
Am I correct?

@OriolAbril
Copy link
Member Author

The problem is not in the examples but in how the type hints are set. A tuple is a perfectly valid way to pass variable names, so would a numpy array be for example. However, the type hint says the input must be a list, instead of having a type hint saying anything of type Sequence is valid.

@utkarsh-maheshwari
Copy link
Contributor

Okay.
I am not very familiar with type-hints. typing_copilot is new.
Can I still take the issue? I will learn about it.

@OriolAbril
Copy link
Member Author

You can still take the issue, I am not very familiar with type hints either. As we were saying on the issue, one of the goals of working on these issues (instead of just ignoring them because they don't affect the actual library) is precisely for all of us to get a bit more familiar with type hints.

@abhisht51
Copy link
Contributor

Hello @OriolAbril, are there more changes to be done to this issue?

@OriolAbril
Copy link
Member Author

As per my comment on #1587: #1586 (comment)

I am realizing that there are many other places where we have inputs type hinted as lists when they should be sequences. I will merge this to go ahead with typing copilot directly with latest mypy but keep the issue open.

So if you know about type hints, it would be great to go over the type hints that are currently in use and make sure they are indeed correct and not too restrictive like this List instead of Sequence example. I am relabeling the issue however because we are no longer targetting some specific issues and the issue has become quite open ended. Moreover, we are still learning about type hinting, so the guidance we can provide on this is quite limited.

I also want to note that this is a low priority issue

@OriolAbril OriolAbril changed the title Fix mypy errors Fix type hints errors Nov 23, 2022
@OriolAbril OriolAbril changed the title Fix type hints errors Fix type hints Nov 23, 2022
@OriolAbril
Copy link
Member Author

We removed the mypy check in CI, so there are no longer mypy errors, but if someone knows about type hints and can help it would be nice to still check the type hints we do have

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

No branches or pull requests

3 participants