Skip to content

Fix mypy errors attributed to pytorch_lightning.loggers.base.py #13494

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 16 commits into from
Jul 12, 2022
Merged

Fix mypy errors attributed to pytorch_lightning.loggers.base.py #13494

merged 16 commits into from
Jul 12, 2022

Conversation

@jxtngx
Copy link
Contributor

@jxtngx jxtngx commented Jul 1, 2022 •
edited
Loading

What does this PR do?

Fixes mypy errors attributed to pytorch_lightning/loggers/base.py for issue #13445

  1. removed module name from pyproject.toml [tool.mypy.overrides]
  2. As per instructions, set return type to Any for:
    • loggers.base.rank_zero_experiment
    • loggers.base.merge_dict

Does your PR introduce any breaking changes? If yes, please list them.

None

Before submitting

  • Was this discussed/approved via a GitHub issue? (not for typos and docs)
  • Did you read the contributor guideline, Pull Request section?
  • Did you make sure your PR does only one thing, instead of bundling different changes together?
  • Did you make sure to update the documentation with your changes? (if necessary)
  • Did you write any new necessary tests? (not for typos and docs)
  • Did you verify new and existing tests pass locally with your changes?
  • Did you list all the breaking changes introduced by this pull request?
  • Did you update the CHANGELOG? (not for typos, docs, test updates, or minor internal changes/refactors)

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:

  • Is this pull request ready for review? (if not, please submit in draft mode)
  • Check that all items from Before submitting are resolved
  • Make sure the title is self-explanatory and the description concisely explains the PR
  • Add labels and milestones (and optionally projects) to the PR so it can be classified

@otaj otaj mentioned this pull request Jul 1, 2022
Closed
52 tasks
carmocca
carmocca reviewed Jul 1, 2022
View reviewed changes
@akihironitta akihironitta added code quality community This PR is from the community labels Jul 3, 2022
@akihironitta akihironitta added this to the pl:1.7 milestone Jul 3, 2022
@awaelchli awaelchli added the logger Related to the Loggers label Jul 4, 2022
@jxtngx jxtngx changed the title Fixes mypy errors attributed to pytorch_lightning/loggers/base.py for issue #13445 Fix mypy errors attributed to pytorch_lightning/loggers/base.py for issue #13445 Jul 5, 2022
@jxtngx jxtngx marked this pull request as draft July 5, 2022 16:30
@jxtngx
Copy link
Contributor Author

jxtngx commented Jul 5, 2022

This PR is in draft until #13541 is resolved.

base.merge_dicts relies on logger.merge_dicts.

mypy errors for logger.merge_dicts must be fixed before base.merge_dicts can be considered fixed.

@jxtngx jxtngx changed the title Fix mypy errors attributed to pytorch_lightning/loggers/base.py for issue #13445 Fix mypy errors attributed to pytorch_lightning/loggers/base.py Jul 8, 2022
@jxtngx jxtngx changed the title Fix mypy errors attributed to pytorch_lightning/loggers/base.py Fix mypy errors attributed to pytorch_lightning.loggers.base.py Jul 9, 2022
@jxtngx jxtngx marked this pull request as ready for review July 11, 2022 12:47
@jxtngx
Copy link
Contributor Author

jxtngx commented Jul 11, 2022 •
edited
Loading

This PR depended on #13541.

13541 was merged into upstream default; I updated my default branch, merged into this PR branch, and made appropriate changes.

I made changes to the docstring notes in logger.rank_zero_experiment to correct errors I had made. No code was changed in that file.

@mergify mergify bot added the ready PRs ready to be merged label Jul 11, 2022
@otaj otaj enabled auto-merge (squash) July 11, 2022 14:29
rohitgr7
rohitgr7 approved these changes Jul 11, 2022
View reviewed changes
auto-merge was automatically disabled July 11, 2022 17:34

Head branch was pushed to by a user without write access

jxtngx and others added 3 commits July 11, 2022 13:34
@carmocca carmocca merged commit 57f5e18 into Lightning-AI:master Jul 12, 2022
jerome-habana pushed a commit to jerome-habana/lightning that referenced this pull request Jul 14, 2022
@jxtngx @rohitgr7
@carmocca @jerome-habana
Fix mypy errors attributed to pytorch_lightning.loggers.base.py (Li…
c1ec00e 
…ghtning-AI#13494)

Co-authored-by: Rohit Gupta <[email protected]>
Co-authored-by: Carlos Mocholí <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@tchaton tchaton Awaiting requested review from tchaton tchaton is a code owner

@Borda Borda Awaiting requested review from Borda Borda is a code owner

@williamFalcon williamFalcon Awaiting requested review from williamFalcon

@SeanNaren SeanNaren Awaiting requested review from SeanNaren

@justusschock justusschock Awaiting requested review from justusschock justusschock is a code owner

@kaushikb11 kaushikb11 Awaiting requested review from kaushikb11

5 more reviewers

@awaelchli awaelchli awaelchli approved these changes

@otaj otaj otaj approved these changes

@carmocca carmocca carmocca approved these changes

@akihironitta akihironitta akihironitta approved these changes

@rohitgr7 rohitgr7 rohitgr7 approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

code quality community This PR is from the community logger Related to the Loggers ready PRs ready to be merged

Projects

None yet

Milestone

pl:1.7

Development

Successfully merging this pull request may close these issues.

6 participants

@jxtngx @awaelchli @otaj @carmocca @akihironitta @rohitgr7