Skip to content

Conversation

@MGAMZ
Copy link
Contributor

@MGAMZ MGAMZ commented Oct 26, 2025

This is a sub-PR of #1665.

This PR fixes a compatibility issue in get_version() caused by relying on exec() to modify the local namespace.

The current implementation executes a version file with exec(...) and then attempts to retrieve __version__ from locals(). While this may appear to work in Python ≤3.12, it breaks in Python 3.13+ due to changes introduced by PEP 667, which standardizes execution context behavior and prevents exec() from mutating a function’s local scope.

To ensure correctness and forward compatibility, this PR explicitly passes a dedicated dictionary as the global namespace to exec(), and safely reads __version__ from that namespace.

Noted that setup.py is deprecated and we should include pyproject.toml according to the latest PyPA, and PEP 621.

@MGAMZ MGAMZ changed the title Fix editable install in newer versions of python [Fix] editable install in python>=3.13 Oct 26, 2025
@MGAMZ
Copy link
Contributor Author

MGAMZ commented Oct 26, 2025

@HAOCHENYE This one ready to be reviewed.

@MGAMZ
Copy link
Contributor Author

MGAMZ commented Oct 26, 2025

This PR fails at some pytest unit due to Network connecting issue or me not having multi-GPUs to test 😂, including:

tests/test_fileio/test_fileclient.py
tests/test_hooks/test_sync_buffers_hook.py
tests/test_model/test_model_utils.py::test_convert_syncbn
tests/test_strategies/test_fsdp.py

And tests/test_visualizer/test_vis_backend.py fails for not having a matched distribution of aimrocks

pytest runs on

  1. python 3.13.9
  2. torch 2.10.0.dev

@MGAMZ MGAMZ marked this pull request as ready for review October 26, 2025 07:59
Copilot AI review requested due to automatic review settings October 26, 2025 07:59
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR addresses a Python 3.13+ compatibility issue in the get_version() function caused by PEP 667 changes to how exec() interacts with local namespaces. The fix explicitly provides a dictionary as the global namespace to exec() and retrieves the version from that dictionary, ensuring the code works correctly across Python versions.

Key Changes:

  • Modified get_version() to use an explicit dictionary for exec() namespace instead of relying on locals()
  • Changed version retrieval from locals()['__version__'] to direct dictionary access

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@HAOCHENYE
Copy link
Collaborator

Leave this PR for review last. I'm considering replacing setuptools with hatch as mmengine's new build system.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants