Skip to content

Lift the Python upper bound - #104

Open
yemine0x01 wants to merge 4 commits into
gridfm:mainfrom
yemine0x01:lift-python-upper-bound
Open

yemine0x01 wants to merge 4 commits into
gridfm:mainfrom
yemine0x01:lift-python-upper-bound

Conversation

@yemine0x01

Copy link
Copy Markdown
Collaborator

No description provided.

Signed-off-by: yemine0x01 <youssouf.emine@artelys.com>
Signed-off-by: yemine0x01 <youssouf.emine@artelys.com>
@romeokienzler

Copy link
Copy Markdown
Collaborator

@yemine0x01 thanks for cleaning up the version story — dropping the stale Windows 3.12 warning and lifting the bound is a nice simplification.

What's needed:

  • CI doesn't yet exercise the newly-claimed versions. .github/workflows/ci-build.yaml runs only on Python 3.12, but this PR advertises 3.13 and 3.14 (both the lifted requires-python = ">=3.10" and the new 3.13/3.14 classifiers). Without a test run on those interpreters we have no evidence the package actually works there. Consider either adding 3.13 (and 3.14 if you want to claim it) to the CI matrix, or trimming the classifiers back to what's tested — an unbounded requires-python will let pip install on interpreters we've never validated.
  • Checks still pending: pytests and dynamic-pytests are running; please make sure they go green before this is picked up.
  • Reviewer: per CONTRIBUTING, please add Alban Puech as a reviewer.

Docs-only otherwise looks clean — no new deps or params, so no YAML/docs-component updates needed. I'll leave the merge call to a maintainer.

— 🤖 _automated pre-review; a maintainer will follow up_

Signed-off-by: yemine0x01 <youssouf.emine@artelys.com>
@romeokienzler

Copy link
Copy Markdown
Collaborator

@yemine0x01 the new pytests-other-pythons matrix job nails the main thing I raised — it now exercises 3.10, 3.11, 3.13 and 3.14 with the full Julia/PowerModels setup, and 3.12 stays covered by the existing pytests job, so every version you claim in the classifiers is actually tested. Nice.

What's left:

  • Let CI go green. Because this is a fork PR the workflow runs need a maintainer to approve them, and the new matrix adds four more legs — please keep an eye on pytests, dynamic-pytests and the new pytests-other-pythons runs and make sure they all pass (3.13/3.14 wheels for the scientific deps are the most likely place something surfaces).
  • Reviewer: still worth adding Alban Puech as a reviewer per CONTRIBUTING.

Otherwise this looks ready for a maintainer's look — I'll leave the merge call to them.

— 🤖 _automated pre-review; a maintainer will follow up_

Signed-off-by: yemine0x01 <youssouf.emine@artelys.com>
@romeokienzler

Copy link
Copy Markdown
Collaborator

@yemine0x01 the new commit bumps the matrix job to Node 24 action versions (checkout@v7, setup-julia@v3, cache@v6, setup-python@v7) — sensible pre-emptive fix so the new legs don't trip the deprecated-Node runner. None of this changes my earlier points, and I don't see anything new broken by it.

One small heads-up: the rest of ci-build.yaml (the existing pytests / dynamic-pytests jobs) is still on checkout@v4 / setup-julia@v1 / cache@v4 / setup-python@v4, so the new job now diverges from its siblings. Not a blocker — the older pins still run — but if you want the file internally consistent you could bump them all in one go (equally fine to leave for a separate cleanup).

Still open from before:

  • CI green: the pytests, dynamic-pytests and new pytests-other-pythons runs still need a maintainer to approve the fork workflow before they execute — only DCO has reported so far. Worth watching the 3.13/3.14 legs once they start.
  • Reviewer: still worth adding Alban Puech per CONTRIBUTING.

Otherwise unchanged — looks ready for a maintainer's look once CI runs.

— 🤖 _automated pre-review; a maintainer will follow up_

This branch has not been deployed

No deployments
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