Skip to content

Avoid reassignment in interface functions#368

Merged
ValerianRey merged 2 commits intomainfrom
use-new-variables
Jun 2, 2025
Merged

Avoid reassignment in interface functions#368
ValerianRey merged 2 commits intomainfrom
use-new-variables

Conversation

@ValerianRey
Copy link
Copy Markdown
Contributor

In backward and mtl_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-redefinition flag 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 using cast, 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 name xyz the original variable and xyz_ the one that has been changed to the right format / type.

@ValerianRey ValerianRey added package: autojac cc: refactor Conventional commit type for any refactoring, not user-facing, and not typing or perf improvements labels May 29, 2025
@ValerianRey ValerianRey self-assigned this May 29, 2025
@ValerianRey ValerianRey mentioned this pull request May 29, 2025
@codecov
Copy link
Copy Markdown

codecov bot commented May 29, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Files with missing lines Coverage Δ
src/torchjd/_autojac/_backward.py 100.00% <100.00%> (ø)
src/torchjd/_autojac/_mtl_backward.py 100.00% <100.00%> (ø)
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@ValerianRey ValerianRey changed the title Create new variables when changing their type Avoid reassignment May 29, 2025
@ValerianRey ValerianRey changed the title Avoid reassignment Avoid reassignment in interface functions May 29, 2025
@ValerianRey ValerianRey mentioned this pull request May 29, 2025
1 task
@ValerianRey ValerianRey requested a review from PierreQuinton May 29, 2025 19:19
@ValerianRey ValerianRey merged commit 0174765 into main Jun 2, 2025
16 checks passed
@ValerianRey ValerianRey deleted the use-new-variables branch June 2, 2025 10:52
@ValerianRey ValerianRey mentioned this pull request Jun 4, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

cc: refactor Conventional commit type for any refactoring, not user-facing, and not typing or perf improvements package: autojac

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants