-
-
Notifications
You must be signed in to change notification settings - Fork 622
Run autotyping on entire codebase #40180
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
base: develop
Are you sure you want to change the base?
Conversation
I'm marking this as a draft because this is an unreasonably large PR, but I'm not sure how to best go about splitting it up. @fchapoton any thoughts? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If you look for "Return type" in https://github.com/sagemath/sage/actions/runs/15310228472/job/43072906204?pr=40180, you see that a few of the added annotations are not corresponding to what the code actually returns at the moment.
I got the impression that most of these are actual minor bugs that should be fixed (eg adding pass
in abstract methods). Not sure though if all of these failures are due to code bugs, or if some of these methods are totally fine to return say int | None
.
@@ -701,7 +701,7 @@ def _repr_term(self, m): | |||
""" | |||
p, v = m | |||
|
|||
def ppr(i): | |||
def ppr(i) -> str: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
/home/runner/work/sage/sage/src/sage/algebras/quantum_clifford.py:704:23 - warning: Function with declared return type "str" must return value on all code paths
Type "None" cannot be assigned to type "str" (reportGeneralTypeIssues)
@@ -766,7 +766,7 @@ class ElementMethods: | |||
# return self == self.parent().zero() | |||
< 8000 /td> |
|
||
@abstract_method | |||
def __bool__(self): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
/home/runner/work/sage/sage/src/sage/categories/additive_magmas.py:769:35 - warning: Function with declared return type "bool" must return value on all code paths
Type "None" cannot be assigned to type "bool" (reportGeneralTypeIssues)
@@ -178,7 +178,7 @@ def dual_code(self): | |||
return self | |||
return super().dual_code() | |||
|
|||
def minimum_distance(self): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
/home/runner/work/sage/sage/src/sage/coding/golay_code.py:181:35 - warning: Function with declared return type "int" must return value on all code paths
Type "None" cannot be assigned to type "int" (reportGeneralTypeIssues)
Yeah, this is an automatic tool and I only gave a quick glance at the output, hence why the PR is marked as a draft. Given how many files this PR touches, do you think it needs to be split up, and if so, how should we split this up? |
In my opinion it would be okay to merge this in its current state (after fixing the trivial doctest failure). The existing typing info is currently not in a state where it would be really authoritative. So it's fine if some of the added typing annotations in this PR are wrong - the important part is that most of them are correct. (Maybe we should add a short paragraph in the dev docs about the current state of the typing info, and what to do in case the typing checker complains.) For the future progress, I would propose to
|
I used https://github.com/JelleZijlstra/autotyping/ to add type annotations for all magic methods and methods that return a basic Python type.
Inspired by @fchapoton's use of autoformatting tools to improve the Sage codebase.
Right now I've done two passes with
autotyping
. The first adds type annotations to all magic methods (__init__
,__len__
, etc.), the second adds type annotations to any method that always returns a basic Python literal (for instance,is_
methods that always return abool
, and_latex_
methods that always return astr
).📝 Checklist