Fix support for dataclasses with ClassVar/InitVar in `apply_to_collection` #9702

a-gardner1 · 2021-09-25T19:10:06Z

What does this PR do?

Adds/fixes support for ClassVar and InitVar fields in dataclasses to apply_to_collection

Fixes #9208

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 internal minor changes/refactorings)

PR review

Anyone in the community is welcome to review the PR.
Before you start reviewing make sure you have read 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

Did you have fun?

Make sure you had fun coding 🙃

for more information, see https://pre-commit.ci

codecov · 2021-09-25T19:31:18Z

Codecov Report

Merging #9702 (fc77d6c) into master (45f6a3b) will decrease coverage by 4%.
The diff coverage is 94%.

❗ Current head fc77d6c differs from pull request most recent head 50db279. Consider uploading reports for the commit 50db279 to get more accurate results

@@           Coverage Diff           @@
##           master   #9702    +/-   ##
=======================================
- Coverage      93%     89%    -4%     
=======================================
  Files         181     181            
  Lines       16370   16381    +11     
=======================================
- Hits        15144   14526   -618     
- Misses       1226    1855   +629

tests/utilities/test_apply_func.py

…pplication_to_collection.

pytorch_lightning/utilities/apply_func.py

tests/utilities/test_apply_func.py

pytorch_lightning/utilities/apply_func.py

CHANGELOG.md

tchaton · 2021-10-19T18:31:11Z

Dear @a-gardner1,

Would you mind resolving the conflicts ?

Best,
T.C

a-gardner1 · 2021-10-21T21:12:11Z

Dear @a-gardner1,

Would you mind resolving the conflicts ?

Best, T.C

Sure. Just a heads-up, though, that I'm unlikely to have time to get back to this before November.

…s-initvar Conflicts resolved: CHANGELOG.md pytorch_lightning/utilities/apply_func.py tests/utilities/test_apply_func.py

carmocca · 2021-11-07T02:10:11Z

Approved the CI to run - you can ping me on Slack if you need it again. It's no longer necessary after the first contribution.

a-gardner1 · 2021-11-08T19:04:08Z

I cannot reproduce the failed test. To the best of my knowledge, it does not use anything that this PR has modified.

Local environment vs failed CI environment:

CentOS 8.2.2004 vs Ubuntu 18.0.4,
Python 3.9.8 vs Python 3.9.7,
torch 1.10.0 vs torch 1.10.0

pytorch_lightning/utilities/apply_func.py

pytorch_lightning/trainer/connectors/logger_connector/result.py

…roup.

…s-initvar Conflicts resolved: CHANGELOG.md

…tion` (#9702) Co-authored-by: Adrian Wälchli <aedu.waelchli@gmail.com> Co-authored-by: Carlos Mocholi <carlossmocholi@gmail.com>

…tion` (Lightning-AI#9702) Co-authored-by: Adrian Wälchli <aedu.waelchli@gmail.com> Co-authored-by: Carlos Mocholi <carlossmocholi@gmail.com>

Add support for ClassVar/InitVar in apply_to_collection.'

8be8315

a-gardner1 requested review from awaelchli, Borda, carmocca, justusschock, kaushikb11, SeanNaren, tchaton and williamFalcon as code owners September 25, 2021 19:10

[pre-commit.ci] auto fixes from pre-commit.com hooks

b87824b

for more information, see https://pre-commit.ci

awaelchli approved these changes Sep 26, 2021

View reviewed changes

tests/utilities/test_apply_func.py Outdated Show resolved Hide resolved

tests/utilities/test_apply_func.py Outdated Show resolved Hide resolved

tests/utilities/test_apply_func.py Outdated Show resolved Hide resolved

awaelchli added the bug Something isn't working label Sep 26, 2021

awaelchli added this to the v1.4.x milestone Sep 26, 2021

awaelchli changed the title ~~Fix support for dataclasses with ClassVar/InitVar in apply_to_collection.'~~ Fix support for dataclasses with ClassVar/InitVar in `apply_to_collection? Sep 26, 2021

awaelchli changed the title ~~Fix support for dataclasses with ClassVar/InitVar in `apply_to_collection?~~ Fix support for dataclasses with ClassVar/InitVar in apply_to_collection Sep 26, 2021

a-gardner1 added 2 commits September 26, 2021 15:49

Reduce duplication in test_recursive_application_to_collection.

7bc5ef5

Replace remaining torch.allclose with torch.equal in test_recursive_a…

363ef86

…pplication_to_collection.

carmocca reviewed Sep 27, 2021

View reviewed changes

pytorch_lightning/utilities/apply_func.py Outdated Show resolved Hide resolved

tests/utilities/test_apply_func.py Show resolved Hide resolved

a-gardner1 commented Sep 27, 2021

View reviewed changes

pytorch_lightning/utilities/apply_func.py Outdated Show resolved Hide resolved

awaelchli approved these changes Sep 28, 2021

View reviewed changes

awaelchli reviewed Sep 28, 2021

View reviewed changes

CHANGELOG.md Outdated Show resolved Hide resolved

awaelchli and others added 2 commits September 28, 2021 11:04

Update CHANGELOG.md

d628792

Avoid use of copy in apply_to_collection.

4656ed8

a-gardner1 requested a review from rohitgr7 as a code owner October 6, 2021 22:53

mergify bot added the has conflicts label Oct 17, 2021

carmocca modified the milestones: v1.4.x, v1.6.x Oct 27, 2021

awaelchli modified the milestones: v1.6.x, 1.5.x Nov 3, 2021

a-gardner1 added 2 commits November 6, 2021 15:41

Merge remote-tracking branch 'origin/master' into wip-apply-func-clas…

003615d

…s-initvar Conflicts resolved: CHANGELOG.md pytorch_lightning/utilities/apply_func.py tests/utilities/test_apply_func.py

Use deepcopy instead of copy in apply_to_collection

05ae33c

mergify bot removed the has conflicts label Nov 6, 2021

Restore skipping of non-init fields after incorrectly resolved conflict.

9130a26

Fix deepcopy of _Sync fns and fix comparison in test_apply_func.

fc77d6c

Minor re-structure

50db279

carmocca approved these changes Nov 9, 2021

View reviewed changes

pytorch_lightning/utilities/apply_func.py Show resolved Hide resolved

pytorch_lightning/trainer/connectors/logger_connector/result.py Outdated Show resolved Hide resolved

mergify bot added the ready PRs ready to be merged label Nov 9, 2021

justusschock approved these changes Nov 9, 2021

View reviewed changes

carmocca self-assigned this Nov 9, 2021

Ensure regeneration of _Sync function when changing reduction op or g…

e5fb468

…roup.

mergify bot added the has conflicts label Nov 9, 2021

a-gardner1 added 2 commits November 9, 2021 17:17

Merge remote-tracking branch 'origin/master' into wip-apply-func-clas…

cbe82a3

…s-initvar Conflicts resolved: CHANGELOG.md

Explicitly retain non-init fields in apply_to_collection.

29a8154

mergify bot removed the has conflicts label Nov 9, 2021

Fix CHANGELOG

d42610b

carmocca enabled auto-merge (squash) November 10, 2021 04:10

carmocca merged commit ce149f6 into Lightning-AI:master Nov 10, 2021

awaelchli added a commit that referenced this pull request Nov 10, 2021

Fix support for dataclasses with ClassVar/InitVar in `apply_to_collec…

f5e3907

…tion` (#9702) Co-authored-by: Adrian Wälchli <aedu.waelchli@gmail.com> Co-authored-by: Carlos Mocholi <carlossmocholi@gmail.com>

lexierule pushed a commit that referenced this pull request Nov 16, 2021

Fix support for dataclasses with ClassVar/InitVar in `apply_to_collec…

4bc6e95

…tion` (#9702) Co-authored-by: Adrian Wälchli <aedu.waelchli@gmail.com> Co-authored-by: Carlos Mocholi <carlossmocholi@gmail.com>

rohitgr7 mentioned this pull request Feb 7, 2022

Weekly Patch Release v1.5.10 #11789

Closed

12 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix support for dataclasses with ClassVar/InitVar in `apply_to_collection` #9702

Fix support for dataclasses with ClassVar/InitVar in `apply_to_collection` #9702

a-gardner1 commented Sep 25, 2021 •

edited by awaelchli

codecov bot commented Sep 25, 2021 •

edited

tchaton commented Oct 19, 2021

a-gardner1 commented Oct 21, 2021

carmocca commented Nov 7, 2021

a-gardner1 commented Nov 8, 2021

Fix support for dataclasses with ClassVar/InitVar in apply_to_collection #9702

Fix support for dataclasses with ClassVar/InitVar in apply_to_collection #9702

Conversation

a-gardner1 commented Sep 25, 2021 • edited by awaelchli

What does this PR do?

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

Before submitting

PR review

Did you have fun?

codecov bot commented Sep 25, 2021 • edited

Codecov Report

tchaton commented Oct 19, 2021

a-gardner1 commented Oct 21, 2021

carmocca commented Nov 7, 2021

a-gardner1 commented Nov 8, 2021

Fix support for dataclasses with ClassVar/InitVar in `apply_to_collection` #9702

Fix support for dataclasses with ClassVar/InitVar in `apply_to_collection` #9702

a-gardner1 commented Sep 25, 2021 •

edited by awaelchli

codecov bot commented Sep 25, 2021 •

edited