Skip to content

Conversation

@mne-bot
Copy link
Contributor

@mne-bot mne-bot commented Jan 26, 2026

Created by spec_zero GitHub action.

Copy link
Member

@larsoner larsoner left a comment

Choose a reason for hiding this comment

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

Good bot 🚀

But really, nice job @tsbinns ! 👏

@larsoner larsoner enabled auto-merge (squash) January 26, 2026 19:42
@larsoner
Copy link
Member

Okay failure in old is legit, need to figure out how to debug it!

@larsoner
Copy link
Member

Okay looking here:

  Collecting numpy>=1.21.0 (from pyvista==0.47.dev0)
    Downloading numpy-2.2.6-cp310-cp310-manylinux_2_17_x86_64.manylinux2014_x86_64.whl.metadata (62 kB)

it looks like we have two things to fix (at least)

  1. We need to assert something somewhere that the min versions of at least a couple things (NumPy, SciPy) stay at their intended min versions. This could be a new tools/github_actions_ensure_old.py or so that could run after the tools/github_actions_install.sh step. It could read the YAML, import some libraries, and assert the expected version matches the installed mod.__version__.
  2. We need to actually fix test_extra and/or the github_actions_install.sh step/script to make sure our old versions don't get uninstalled. In particular I don't think test_extra should install a dev version of pyvista... I think that was probably a workaround we needed at some point that we can now shouldn't need.

I'll try to push some commits for this...

@larsoner larsoner disabled auto-merge January 26, 2026 21:48
@larsoner larsoner marked this pull request as draft January 26, 2026 21:48
@larsoner
Copy link
Member

Oh... we have no code for automatically updating environment_old.yml! I'll update it manually for now but we should fix this...

@larsoner larsoner marked this pull request as ready for review January 27, 2026 02:40
@larsoner larsoner requested a review from jasmainak as a code owner January 27, 2026 02:40
@tsbinns
Copy link
Contributor

tsbinns commented Jan 27, 2026

Oh... we have no code for automatically updating environment_old.yml! I'll update it manually for now but we should fix this...

This would be addressed in #13490 when it lands, but is still a WIP

@larsoner
Copy link
Member

In the meantime this should be good for review @drammock , I'll fix CIs shortly but the main ideas are in here!

)
for key in opt_dependencies:
opt_dependencies[key] = update_specifiers(opt_dependencies[key], package_releases)
pyproject_data["project"]["dependencies"] = core_dependencies
Copy link
Member

Choose a reason for hiding this comment

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

@tsbinns the old code both updated inplace and set vars, which is unnecessary (and led to some confusion where I assumed things were not updated inplace because the vars were set). I've changed it just to operate inplace where possible.

@larsoner
Copy link
Member

@tsbinns actually I think this would be a good one for you to review + merge if you're happy!

@tsbinns
Copy link
Contributor

tsbinns commented Jan 27, 2026

@larsoner Sure, I'll have a look!

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.

4 participants