Skip to content

Conversation

@baogorek
Copy link
Collaborator

@baogorek baogorek commented Apr 21, 2025

Now leaving Python 3.11 as it lies in the .github folder, but still adding the 3.12 matrix test.

@baogorek baogorek changed the title Enable workflow_dispatch for pr_code_changes Make policyengine-us-data compatible with Python 3.12+ Apr 21, 2025
run: pip install .
- name: Test basic import
run: python -c "import policyengine_us_data; print('Minimal import OK')"
- name: Test specific failing import
Copy link
Collaborator

Choose a reason for hiding this comment

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

question: What is the purpose of this test? Alternatively, why does this import fail, and when?

And could you add the answer as a comment above it, as well?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

The reasoning for this less common test is that the package is currently unusable when installed normally via PyPi or pip install -e . with Python 3.12 (paired with Ubuntu 24.02) but the PR pipeline doesn't catch it. That's because the dev dependencies include setuptools, which is needed for imports such as:

from policyengine_core.data import Dataset

and even

import policyengine_us_data

The latter is one of the explicit tests, so it bothered me that the PR pipeline wouldn't fail, even when the version was changed to 3.12 in the pipeline.

What do you think? If you're good with the test, I'll add some comments.

Copy link
Collaborator

@anth-volk anth-volk left a comment

Choose a reason for hiding this comment

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

Thanks for your work on this @baogorek. I've left some commentary; curious to hear your thoughts.

Separately, the test failure you're seeing may be solved by #208, which is awaiting review

Copy link
Contributor

@nikhilwoodruff nikhilwoodruff left a comment

Choose a reason for hiding this comment

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

Looks good after slight adjustments (and attempt of matrix to reduce duplication)

  • Remove requirement for GITHUB_TOKEN

@baogorek baogorek changed the title Make policyengine-us-data compatible with Python 3.12+ Improved CI/CD pipeline, added dependencies, and other miscellaneous improvements May 28, 2025
@nikhilwoodruff nikhilwoodruff merged commit 3af6fce into main May 28, 2025
12 of 14 checks passed
@nikhilwoodruff nikhilwoodruff deleted the fix/210-add-python312-ci branch May 28, 2025 07:46
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.

policyengine-us-data triggers distuitls error with Python 3.12 when dev dependencies are not installed, and CI/CD does not catch it

4 participants