Skip to content

Avoid inadvertent submodule setup (2/2)#3025

Open
copybara-service[bot] wants to merge 1 commit into
mainfrom
test_524020065
Open

Avoid inadvertent submodule setup (2/2)#3025
copybara-service[bot] wants to merge 1 commit into
mainfrom
test_524020065

Conversation

@copybara-service

@copybara-service copybara-service Bot commented Apr 14, 2023

Copy link
Copy Markdown

Avoid inadvertent submodule setup (2/2)

repr() is often used as development tool, e.g. by IntelliJ debugger window or in logging.

But as it stands, repr() causes inadvertent setup() of submodules with the following code flow:
repr()
-> _module_repr()
-> _attr_repr(submodule)
-> getattr(submodule, 'name', None).
(in submodule)
-> getattr()
-> _try_setup()

When using IntelliJ, repr() eventually try_setup() on a submodule but without proper context. The error is silently eaten in IntelliJ debugger but changes the internal setup state to be DONE and later setup attempt gets ignored, causing failures at a later step.

This CL simply prevents name to not cause try_setup.

@codecov-commenter

codecov-commenter commented Apr 14, 2023

Copy link
Copy Markdown

Codecov Report

Merging #3025 (dc5ef3f) into main (733423a) will increase coverage by 0.01%.
The diff coverage is 90.90%.

@@            Coverage Diff             @@
##             main    #3025      +/-   ##
==========================================
+ Coverage   81.97%   81.99%   +0.01%     
==========================================
  Files          55       55              
  Lines        6002     6019      +17     
==========================================
+ Hits         4920     4935      +15     
- Misses       1082     1084       +2     
Impacted Files Coverage Δ
flax/core/__init__.py 100.00% <ø> (ø)
flax/training/checkpoints.py 68.40% <85.71%> (-0.02%) ⬇️
flax/core/frozen_dict.py 95.68% <92.30%> (-0.35%) ⬇️
flax/linen/module.py 91.89% <100.00%> (+0.01%) ⬆️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

repr() is often used as development tool, e.g. by IntelliJ debugger window or in logging.

But as it stands, repr() causes inadvertent setup() of submodules with the following code flow:
repr()
-> _module_repr()
-> _attr_repr(submodule)
-> getattr(submodule, '__name__', None).
(in submodule)
-> __getattr__()
-> _try_setup()

When using IntelliJ, repr() eventually try_setup() on a submodule but without proper context. The error is silently eaten in IntelliJ debugger but changes the internal setup state to be DONE and later setup attempt gets ignored, causing failures at a later step.

This CL simply prevents __name__ to not cause try_setup.

PiperOrigin-RevId: 524020065
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant