mne-python icon indicating copy to clipboard operation
mne-python copied to clipboard

[MRG] Fix missing safety around device_info field

Open mscheltienne opened this issue 1 year ago • 3 comments

Almost all fields are safeguard behind an if dict.get(...) is not None, and yet I manage to hit one of the only one that was not during saving of a RawArray in a unit test ;)

mscheltienne avatar Aug 27 '24 11:08 mscheltienne

I'm don't know how to get CIs green temporary with the quantities/neo issues.

  • quantities 0.16.0 is the first version compatible with numpy 2.0
  • quantities 0.16.0 is not compatible with numpy dev
  • neo uses a deprecated argument in quantities 0.16.0
  • conda recipe for quantities is still at 0.15.0 thus if I ignore the deprecation warnings issued in neo, the jobs running pip work while the jobs running conda and numpy 2+ fail.

mscheltienne avatar Aug 27 '24 13:08 mscheltienne

  • conda recipe for quantities is still at 0.15.0 thus if I ignore the deprecation warnings issued in neo, the jobs running pip work while the jobs running conda and numpy 2+ fail.

I guess we need to wait for quantities 0.16.1, then bug them to update the recipe (and also either wait for a neo fix, or catch/filter the depr warning)

drammock avatar Aug 27 '24 16:08 drammock

Sorry about this from the Neo side. We wanted to get quantities working so we could focus on numpy 2.0 compatibility, but then forgot to check our own tests. I'll try to push forward on this ASAP. And see what I can do. I don't have any say in the quantities release schedule though.

EDIT: Here's my PR to limit quantities version: so as long as the rest of the team is good with it hopefully we merge this soon!

conda recipe for quantities is still at 0.15.0 thus if I ignore the deprecation warnings issued in neo, the jobs running pip work while the jobs running conda and numpy 2+ fail.

For Neo it usually takes our conda recipe ~1 week to update. So that should be updated within a couple weeks at max as long as Andrew is around to deal with that one.

zm711 avatar Aug 27 '24 18:08 zm711

Owee, I'm MrMeeseeks, Look at me.

There seem to be a conflict, please backport manually. Here are approximate instructions:

  1. Checkout backport branch and update it.
git checkout maint/1.7
git pull
  1. Cherry pick the first parent branch of the this PR on top of the older branch:
git cherry-pick -x -m1 f27bac8b5a128667d7f946f0089132fa5d0da9ba
  1. You will likely have some merge/cherry-pick conflict here, fix them and commit:
git commit -am 'Backport PR #12815: [MRG] Fix missing safety around device_info field'
  1. Push to a named branch:
git push YOURFORK maint/1.7:auto-backport-of-pr-12815-on-maint/1.7
  1. Create a PR against branch maint/1.7, I would have named this PR:

"Backport PR #12815 on branch maint/1.7 ([MRG] Fix missing safety around device_info field)"

And apply the correct labels and milestones.

Congratulations — you did some good work! Hopefully your backport PR will be tested by the continuous integration and merged soon!

Remember to remove the Still Needs Manual Backport label once the PR gets merged.

If these instructions are inaccurate, feel free to suggest an improvement.

lumberbot-app[bot] avatar Aug 31 '24 10:08 lumberbot-app[bot]