Skip to content

Change syncing of dependencies with README#13832

Merged
larsoner merged 10 commits intomne-tools:mainfrom
tsbinns:update_readme_deps
Apr 10, 2026
Merged

Change syncing of dependencies with README#13832
larsoner merged 10 commits intomne-tools:mainfrom
tsbinns:update_readme_deps

Conversation

@tsbinns
Copy link
Copy Markdown
Contributor

@tsbinns tsbinns commented Apr 9, 2026

Closes #13830

Removes the pre-commit hook for copying dependency updates to the README that only runs on release, and instead adds it to the SPEC-0 version bumping action.

Also removes the unused environment.yml file which was superseded by pylock.ci-old.toml.

@larsoner Minor thing, but should sync_dependencies.py be moved from tools/hooks/ to tools/ dir if it's no longer a hook?

Also, thanks for updating the SPEC-0 PR message.

@tsbinns tsbinns marked this pull request as draft April 9, 2026 19:07
@larsoner
Copy link
Copy Markdown
Member

larsoner commented Apr 9, 2026

@larsoner Minor thing, but should sync_dependencies.py be moved from tools/hooks/ to tools/ dir if it's no longer a hook?

Yeah sure, git mv is cheap 😄

Copy link
Copy Markdown
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.

Still in draft mode... @tsbinns do you want to temporarily have a on: pr in spec_zero.yml? We could

  1. Add on: pr trigger
  2. If you do it before #13831 lands you should see a failure saying it tried to create a PR that it couldn't. And the diff from that should match #13831.
  3. Then once #13831 lands , rerun the action, and it should succeed (no changes needed)
  4. Remove on: pr trigger
  5. Merge

@tsbinns
Copy link
Copy Markdown
Contributor Author

tsbinns commented Apr 9, 2026

Error in #13831 for a pip-pre job, so have some breathing room to test on: pr

@tsbinns tsbinns marked this pull request as ready for review April 9, 2026 19:39
@tsbinns
Copy link
Copy Markdown
Contributor Author

tsbinns commented Apr 9, 2026

@larsoner
Copy link
Copy Markdown
Member

larsoner commented Apr 9, 2026

Failure mode is wrong though, it should fail at the "open PR" step:

+ git add doc/changes/dev/dependency.rst
fatal: pathspec 'doc/changes/dev/dependency.rst' did not match any files

it's trying to add a file that is no longer guaranteed to be there

@tsbinns
Copy link
Copy Markdown
Contributor Author

tsbinns commented Apr 9, 2026

Ah good catch.

But, should changes coming from syncing the existing dependencies to README need a changelog entry? If these are dependency changes not coming from the SPEC0 action, but from an old PR that is just getting the versions synced to README, would we rely on the dep changes being documented in the original PR that introduced them?

I could try changing the logic that if the only changes are introduced by tools/sync_dependencies.py, we don't try to commit this doc/changes/dev/dependency.rst file.

@larsoner
Copy link
Copy Markdown
Member

I could try changing the logic that if the only changes are introduced by tools/sync_dependencies.py, we don't try to commit this doc/changes/dev/dependency.rst file.

Sure -- I would just wrap it in an if like

if [ -f doc/changes/dependencies.rst ] then 
    git add doc/changes/dependencies.rst
fi

you could do something more complex like check to see if sync_dependencies.py or another step actually made changes but it seems like overkill

@tsbinns
Copy link
Copy Markdown
Contributor Author

tsbinns commented Apr 10, 2026

Yeah, actually gets to PR submission stage this time

@larsoner
Copy link
Copy Markdown
Member

Great! If you can push a commit to remove the on: pr (with a [ci skip] in the commit message) I'll merge without waiting for reqs

Co-authored-by: Eric Larson <larson.eric.d@gmail.com>
@larsoner
Copy link
Copy Markdown
Member

Never mind, easy enough with the UI

Thanks @tsbinns !

@larsoner larsoner merged commit f7cd2d7 into mne-tools:main Apr 10, 2026
11 of 15 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

README dependencies not in sync with pyproject.toml

2 participants