Skip to content
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

Allow for compatibility with Python 3.12 #969

Merged
merged 13 commits into from
Aug 20, 2024
Merged

Conversation

jdebacker
Copy link
Member

This PR updates test files so that the unit tests pass on Python 3.12. As noted in Issue #889, the OG-Core has run on Python 3.12 for some time. The update here ensure that we can tests against 3.12 and therefore publish packages compatible with that version of Python.

cc @rickecon

@jdebacker
Copy link
Member Author

Remaining work:

  • I am aware of a test failure in the test_output_tables.py function. It seems that the new 3.12 pickle from has just one industry whereas the TPI output read into the test_output_tables.test_dynamic_revenue_decomposition test has three industries. TODO: update either the 3.12 pickle to have M=3.

@codecov-commenter
Copy link

codecov-commenter commented Aug 16, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 69.98%. Comparing base (8bbaa26) to head (118479e).
Report is 14 commits behind head on master.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #969      +/-   ##
==========================================
- Coverage   71.08%   69.98%   -1.11%     
==========================================
  Files          20       20              
  Lines        4977     4977              
==========================================
- Hits         3538     3483      -55     
- Misses       1439     1494      +55     
Flag Coverage Δ
unittests 69.98% <100.00%> (-1.11%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files Coverage Δ
ogcore/__init__.py 100.00% <100.00%> (ø)

... and 3 files with indirect coverage changes

@rickecon
Copy link
Member

@jdebacker. The black formatting test is failing. And we should update the middle number (minor) of the version for this one (e.g., 0.12.0). And I think we should make the version requirement python>= 3.7.7, <3.13 in both environment.yml and setup.py. And finally, if we move to Python 3.12 as our latest supported version, I think we should change our GitHub Actions to run on Python 3.11 and 3.12, with the single runs running on 3.12.

@rickecon
Copy link
Member

@jdebacker.

  • The four tests at the end of test_output_tables.py are failing in MacOS Python 3.12, but not in Python 3.10 and 3.11. I thought this was fixed with your previous commits. So I don't understand why it is failing now.
  • In build_and_test.yml, I think we should only test two versions of Python. So I propose we update the tests to only by Python 3.11 and 3.12.
  • In environment.yml and setup.py, I think we should limit the Python version to be less than Python 3.13 (python<=3.7.7,<3.13`). This will prevent the problems that inevitably arise when someone's conda package allows Python 3.13. This page shows the production schedule and expiration dates of Python packages. I think this highlights the value of just keeping two packages that we test on. Note that Python 3.10 is set to expire on 2026-10.
  • In check_black.yml, deploy_docs.yml, docs_check.yml, and publish_to_pypi.yml, update the Python version line to python-version: "3.12".
  • In setup.py, __init__.py, and CHANGELOG.md, update the version to 0.12.0.

@jdebacker jdebacker marked this pull request as ready for review August 20, 2024 15:00
@rickecon
Copy link
Member

@jdebacker. This looks great. Thanks for making this update. Merging now.

@rickecon rickecon merged commit cdb21cb into PSLmodels:master Aug 20, 2024
8 checks passed
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.

3 participants