Skip to content

Fix MaxText pyconfig import in page_manager_test #1705

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.

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

Merged
merged 1 commit into from
May 8, 2025

Conversation

bvandermoon
Copy link
Collaborator

@bvandermoon bvandermoon commented May 8, 2025

Description

Saw this error while running python3 -m pytest on a local TPU VM:

ImportError while importing test module '/home/bvandermoon/workspace/nnx_maxtext/maxtext/MaxText/tests/inference/page_manager_test.py'.
Hint: make sure your test modules/packages have valid Python names.
Traceback:
MaxText/tests/inference/page_manager_test.py:21: in <module>
    import pyconfig
E   ModuleNotFoundError: No module named 'pyconfig'

Updating this import let the tests start locally

Tests

After this change, I was able to run the unit tests on a TPU VM with this:

python3 -m pytest
python3 -m pytest MaxText/tests/inference/page_manager_test.py # Tests passed

Checklist

Before submitting this PR, please make sure (put X in square brackets):

  • I have performed a self-review of my code.
  • I have necessary comments in my code, particularly in hard-to-understand areas.
  • I have run end-to-end tests tests and provided workload links above if applicable.
  • I have made or will make corresponding changes to the doc if needed.

Copy link
Collaborator

@khatwanimohit khatwanimohit left a comment

Choose a reason for hiding this comment

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

I think there's a confusion here. The import pyconfig on line 21 of MaxText/tests/inference/paged_attention_test.py acutally is not the pypi package. It is MaxText/pyconfig.py. The import in that test should be made from MaxText import pyconfig.

@bvandermoon
Copy link
Collaborator Author

I think there's a confusion here. The import pyconfig on line 21 of MaxText/tests/inference/paged_attention_test.py acutally is not the pypi package. It is MaxText/pyconfig.py. The import in that test should be made from MaxText import pyconfig.

Updated, thank you @khatwanimohit

@bvandermoon bvandermoon changed the title Add pyconfig to requirements.txt file for unit tests Fix MaxText pyconfig import in page_manager_test May 8, 2025
@patemotter
Copy link
Collaborator

Thanks for catching/fixing this.

@copybara-service copybara-service bot merged commit e22ad3a into main May 8, 2025
34 of 37 checks passed
@copybara-service copybara-service bot deleted the bvandermoon-nnx branch May 8, 2025 23:57
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.

3 participants