Skip to content

Support Python 3.14#796

Merged
mikemhenry merged 20 commits into
mainfrom
test-py314
Jun 18, 2026
Merged

Support Python 3.14#796
mikemhenry merged 20 commits into
mainfrom
test-py314

Conversation

@mikemhenry

Copy link
Copy Markdown
Contributor

Tips

  • Comment "pre-commit.ci autofix" to have pre-commit.ci atomically format your PR.
    Since this will create a commit, it is best to make this comment when you are finished with your work.

Checklist

  • Added a news entry

Developers certificate of origin

@codecov

codecov Bot commented Jun 5, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 98.54%. Comparing base (e15ae9b) to head (2908298).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main     #796   +/-   ##
=======================================
  Coverage   98.54%   98.54%           
=======================================
  Files          43       43           
  Lines        2810     2811    +1     
=======================================
+ Hits         2769     2770    +1     
  Misses         41       41           

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@mikemhenry

Copy link
Copy Markdown
Contributor Author

Getting these errors now:

FAILED gufe/tests/test_models.py::TestBoxVectors::test_valid_box_quantity[value0-m_expected0] - pydantic_core._pydantic_core.ValidationError: 1 validation error for BoxSettingsModel
box_vectors
  Extra inputs are not permitted [type=extra_forbidden, input_value=<Quantity([[0. 0. 1.]

We are just missing ConfigDict(extra='ignore') in our dummy settings.

@mikemhenry

Copy link
Copy Markdown
Contributor Author

Ah our old friend:

FAILED gufe/tests/test_serialization_json.py::TestPathCodec::test_default - AssertionError: assert {':is_custom:...h': 'foo/bar'} == {':is_custom:...h': 'foo/bar'}
  
  Omitting 3 identical items, use -vv to show
  Differing items:
  {'__module__': 'pathlib'} != {'__module__': 'pathlib._local'}
  
  Full diff:
    {
        ':is_custom:': True,
        '__class__': 'PosixPath',
  -     '__module__': 'pathlib._local',
  ?                           -------
  +     '__module__': 'pathlib',
        'path': 'foo/bar',
    }

@atravitz

atravitz commented Jun 9, 2026

Copy link
Copy Markdown
Contributor

Ah our old friend:

FAILED gufe/tests/test_serialization_json.py::TestPathCodec::test_default - AssertionError: assert {':is_custom:...h': 'foo/bar'} == {':is_custom:...h': 'foo/bar'}
  
  Omitting 3 identical items, use -vv to show
  Differing items:
  {'__module__': 'pathlib'} != {'__module__': 'pathlib._local'}
  
  Full diff:
    {
        ':is_custom:': True,
        '__class__': 'PosixPath',
  -     '__module__': 'pathlib._local',
  ?                           -------
  +     '__module__': 'pathlib',
        'path': 'foo/bar',
    }

First battle with this was here: https://github.com/OpenFreeEnergy/gufe/pull/577/changes

If I understand correctly, pathlib._local is only a python 3.13 thing? In which case we should be able to change this line to if sys.version_info[:2] == (3, 13):

@mikemhenry

Copy link
Copy Markdown
Contributor Author

You are correct, we need to fix that test since this is the current situation:

Runtime writing JSON observed module
Python ≤3.12 pathlib
Python 3.13 pathlib._local
Python 3.14 pathlib

For the test, we can just get the module we need at runtime:

self.dcts = [
    {
        "__class__": "PosixPath",
        "__module__": pathlib.PosixPath.__module__,
        ":is_custom:": True,
        "path": "foo/bar",
    }
]

We can also set up the codec to just use the public module name, something like:

PATH_CODEC = JSONCodec(
    cls=pathlib.PosixPath,
    to_dict=lambda p: {
        "__module__": "pathlib",
        "__class__": "PosixPath",
        ":is_custom:": True,
        "path": str(p),
    },
    from_dict=lambda dct: pathlib.PosixPath(dct["path"]),
    is_my_dict=is_legacy_path_dict,
)

Then the reader will accept ("pathlib", "pathlib._local") but we will always write pathlib even on python 3.13

@atravitz

atravitz commented Jun 9, 2026

Copy link
Copy Markdown
Contributor

Then the reader will accept ("pathlib", "pathlib._local") but we will always write pathlib even on python 3.13

We originally decided not to do this because we anticipated pathlib._local to be the behavior for the future and for pathlib to be legacy, but that turned out not to be the case.

I'm typically inclined to keep the code as-is so that we're not arbitrarily changing things, but since the 3.13 behavior was immediately reverted in 3.14, I'm fine with updating the code to treat 3.13 as an anomaly that we're accounting for and update the codec.

tl;dr: yeah do it 🚀

@mikemhenry

Copy link
Copy Markdown
Contributor Author

Sweet, looking at the issues and mailing lists, it looks like it was a mistake that happened during a re-factor which is why 3.13 is an anomaly

tl;dr 🚀

@mikemhenry mikemhenry changed the title Test Python 3.14 Support Python 3.14 Jun 16, 2026
@mikemhenry

Copy link
Copy Markdown
Contributor Author

The API check bot is having an issue, will fix later ImportError: With sys.path = ['/tmp/griffe-worktree-gufe-origin-main-e4757g4_/origin-main/src/gufe'], accessing 'gufe' raises ModuleNotFoundError: No module named 'gufe'

@atravitz atravitz left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

some questions

Comment thread pyproject.toml Outdated
Comment thread src/gufe/serialization/json.py Outdated
Comment thread src/gufe/serialization/json.py Outdated
Comment thread src/gufe/settings/models.py
@mikemhenry mikemhenry requested a review from atravitz June 17, 2026 18:13
Comment thread src/gufe/serialization/json.py Outdated

@atravitz atravitz left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

just one docstring correction, then good to merge!

Co-authored-by: Alyssa Travitz <31974495+atravitz@users.noreply.github.com>
@github-actions

Copy link
Copy Markdown

🚨 API breaking changes detected! 🚨
View logs for this step

@mikemhenry mikemhenry enabled auto-merge (squash) June 18, 2026 17:00
@mikemhenry mikemhenry merged commit ea2c69e into main Jun 18, 2026
16 checks passed
@mikemhenry mikemhenry deleted the test-py314 branch June 18, 2026 17:04
@mikemhenry mikemhenry restored the test-py314 branch June 23, 2026 16:05
@mikemhenry mikemhenry deleted the test-py314 branch June 23, 2026 16:40
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.

2 participants