[WIP] Adopt PEP 563, 585, and 604 for src/lightning/pytorch #17779
Conversation
| @@ -11,9 +11,12 @@ | |||
| # WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | |||
| # See the License for the specific language governing permissions and | |||
| # limitations under the License. | |||
| from lightning.pytorch.loops.loop import _Loop # noqa: F401 isort: skip (avoids circular imports) | |||
There was a problem hiding this comment.
Moving this is what is causing the circular import issues. I assume one of the tools did it accidentally? isort should be skipping it
There was a problem hiding this comment.
I ran with isort version 5.10.1. Maybe that was the issue.
src/lightning/pytorch/__init__.py
Outdated
| @@ -1,5 +1,7 @@ | |||
| """Root package info.""" | |||
|
|
|||
| from __future__ import annotations | |||
There was a problem hiding this comment.
We'd need to remove all these unused additions. Maybe we can write a script that processes the git diff and undoes it if this is the only change
There was a problem hiding this comment.
for f in $(git diff --name-only master..HEAD); do
if [ "$(git diff master..HEAD $f | sed -n '/^+++/d; /^+$/d; /^+/p')" = "+from __future__ import annotations" ]; then
git checkout master -- $f
fi
done
|
For a draft pull request, the |
There was a problem hiding this comment.
In the doctests there is one failure, but not jsonargparse related. It fails for torch.jit.script.
There was a problem hiding this comment.
There was a problem hiding this comment.
This seems like a deal breaker since pytorch is central to lightning. The postponed annotations work correctly with jsonargparse 4.22.0. There are some unit tests in test_cli.py that fail but it is because of mocking. For now I think I will just close this pull request.
| @@ -44,7 +46,7 @@ def clean_namespace(hparams: MutableMapping) -> None: | |||
| del hparams[k] | |||
|
|
|||
|
|
|||
| def parse_class_init_keys(cls: Type) -> Tuple[str, Optional[str], Optional[str]]: | |||
| def parse_class_init_keys(cls: type) -> tuple[str, str | None, str | None]: | |||
There was a problem hiding this comment.
mypy does not consider Type and type equivalent?
carmocca
changed the title
carmocca
requested review from
williamFalcon,
tchaton,
awaelchli,
justusschock,
lantiga and
Borda
as code owners
They don't. I marked the pull request as ready and added WIP to the title. |
What does this PR do?
Try to pyupgrade src/lightning/pytorch to adopt PEP 563, 585, and 604. Ran the following and then manually fixed
src/lightning/pytorch/loggers/neptune.pyPart of #11205
Before submitting
PR review
Anyone in the community is welcome to review the PR.
Before you start reviewing, make sure you have read the review guidelines. In short, see the following bullet-list:
Reviewer checklist