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 mypy errors #1145

Merged
merged 16 commits into from
Oct 26, 2024
Merged

Fix mypy errors #1145

merged 16 commits into from
Oct 26, 2024

Conversation

davidar
Copy link
Collaborator

@davidar davidar commented Oct 26, 2024

I've been going through fixing mypy errors, and have gotten through about half of them. In the process I've added some extra error handling that fixes some exceptions I've seen due to methods being called on the wrong types.

Current master:

Found 1718 errors in 183 files (checked 443 source files)

This branch:

Found 872 errors in 155 files (checked 443 source files)

Feel free to squash all these commits, the only reason I've left them separate is that I made sure the test suite was passing after each one.

@rocky
Copy link
Member

rocky commented Oct 26, 2024

Reducing mypy errors has long been a desire. So I really appreciate this.

The main ask is to be sensitive as to when to use a list versus a tuple. I don't think one can know in advance whether the disruption is too great. Too often in this code base the mess is too great that we can't tackle an entire problem in one go. Instead, personally I have found that for me, I have to do it little by little.

Also, if you would like direct write access to the repositories, let us know.

@@ -10,6 +10,7 @@

import typing
from itertools import permutations
from typing import Optional, Tuple
Copy link
Member

Choose a reason for hiding this comment

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

Thanks!

@rocky
Copy link
Member

rocky commented Oct 26, 2024

LGTM. This is a large PR, in the future smaller ones would be nice. @mmatera - your comments?

@davidar
Copy link
Collaborator Author

davidar commented Oct 26, 2024

LGTM. This is a large PR, in the future smaller ones would be nice.

Yeah, sorry about that, I couldn't really find a neat way to cleave it up so I kind of just kept going until I got bored of it...

Also, if you would like direct write access to the repositories, let us know.

Sure, if that would make it easier to make these kinds of changes incrementally?

@rocky
Copy link
Member

rocky commented Oct 26, 2024

LGTM. This is a large PR, in the future smaller ones would be nice.

Yeah, sorry about that, I couldn't really find a neat way to cleave it up so I kind of just kept going until I got bored of it...

Also, if you would like direct write access to the repositories, let us know.

Sure, if that would make it easier to make these kinds of changes incrementally?

An invite should have been sent.

Copy link
Contributor

@mmatera mmatera left a comment

Choose a reason for hiding this comment

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

Very nice! Thank you very much! LGTM

@rocky rocky merged commit 6212284 into Mathics3:master Oct 26, 2024
12 checks passed
@davidar davidar deleted the typing branch October 27, 2024 00:52
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.

3 participants