Skip to content

Conversation

QEDady
Copy link

@QEDady QEDady commented Oct 9, 2025

Currently, reloading a not-yet materialized lazy module fails. This PR changes it to be a no-op. This makes sense since the reloading will happen anyway when the module is materialized e.g. by accessing an attribute.

QEDady added 2 commits October 9, 2025 12:15
… in test_lazy.py

Without this `./python -m test test_importlib` fails at `test_lazy`
although `./python -m test test_importlib.test_lazy` perfectyl succeeds.
That is becasue some other tests likely manipulates `util` making the
above two out of sync.
@bedevere-app
Copy link

bedevere-app bot commented Oct 9, 2025

Most changes to Python require a NEWS entry. Add one using the blurb_it web app or the blurb command-line tool.

If this change has little impact on Python users, wait for a maintainer to apply the skip news label instead.

@python-cla-bot
Copy link

python-cla-bot bot commented Oct 9, 2025

All commit authors signed the Contributor License Agreement.

CLA signed

@bedevere-app
Copy link

bedevere-app bot commented Oct 9, 2025

Most changes to Python require a NEWS entry. Add one using the blurb_it web app or the blurb command-line tool.

If this change has little impact on Python users, wait for a maintainer to apply the skip news label instead.

Copy link
Member

@brettcannon brettcannon left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minor tweaks to the tests, but otherwise LGTM!

@bedevere-app
Copy link

bedevere-app bot commented Oct 9, 2025

A Python core developer has requested some changes be made to your pull request before we can consider merging it. If you could please address their requests along with any other requests in other reviews from core developers that would be appreciated.

Once you have made the requested changes, please leave a comment on this pull request containing the phrase I have made the requested changes; please review again. I will then notify any core developers who have left a review that you're ready for them to take another look at this pull request.

Copy link
Author

@QEDady QEDady left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have made the requested changes; please review again

@QEDady
Copy link
Author

QEDady commented Oct 10, 2025

I have made the requested changes; please review again

Also, currently the following test:

./python -m test test.test_os.test_os

fails with

test test.test_os.test_os failed -- Traceback (most recent call last):
  File "/usr/local/google/home/amere/cpython/Lib/test/test_os/test_os.py", line 1011, in test_utime_fd
    self._test_utime(set_time)
    ~~~~~~~~~~~~~~~~^^^^^^^^^^
  File "/usr/local/google/home/amere/cpython/Lib/test/test_os/test_os.py", line 920, in _test_utime
    self.assertAlmostEqual(st.st_atime, atime_ns * 1e-9, delta=1e-6)
    ~~~~~~~~~~~~~~~~~~~~~~^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
AssertionError: 1760084197.4864256 != 1.002003 within 1e-06 delta (1760084196.4844227 difference)

because of

sys.modules['importlib.util'] = util

at the top level of the test_lazy.py. Removing the latter makes ./python -m test test_importlib fails with test_lazy failure although ./python -m test test_importlib.test_lazy works fine in all cases. That is because some other test likely reloads importlib.util. See here. Please, advise.

@bedevere-app
Copy link

bedevere-app bot commented Oct 10, 2025

Thanks for making the requested changes!

@brettcannon: please review the changes made to this pull request.

@bedevere-app bedevere-app bot requested a review from brettcannon October 10, 2025 08:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants