Avoid reassignment in interface functions #368
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Closed
Codecov ReportAll modified and coverable lines are covered by tests ✅
🚀 New features to boost your workflow:
|
PierreQuinton
approved these changes
Jun 2, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
In
backwardandmtl_backward, we're defining an interface for the users. This means that we accept many different types for the parameters, and we then change their type to use our internal machinery with the required types. This is not something that mypy likes, because it leads to assignment errors. We could use the--allow-redefinitionflag of mypy, but I think they're kind of right in not allowing redefinitions by default. With redefinitions, it's harder to read the code (since variable types change throughout the same function) and it makes refactors harder (because you may want to rename just the variable after it changed type for instance). We could also solve it usingcast, but it's more or less the same as allowing the redefinition but just locally: it tells mypy "trust me, the type is [...]". So I think the cleanest solution is to use different variables after type has changed. Naming them can be challenging, because we don't like to have type included in the name, but at the same time, in these interfaces, we have two types for the same thing. My solution is to have a new convention to namexyzthe original variable andxyz_the one that has been changed to the right format / type.