Fix pooch

[drammock]

Fixes a variety of problems with the implementation of fetch_dataset(). Also updates the version number and hash of the MNE-misc dataset, which just got a new release today.

[larsoner]

Also updates the version number and hash of the MNE-misc dataset, which just got a new release today.

This will conflict with #9800, but @alexrockhill is an expert merger at this point so I think it's okay :)

[larsoner]

I don't quite get this one. It means that before if the config for the given key did not exist, we would use MNE_DATA, but now we only do so if key is None? This seems like an incorrect change in behavior?

[drammock]

None is a sentinel value for get_config() that returns all config keys and their values that are defined in the config file. But here, key being None actually means something different (viz, "this dataset is MNE-external and is not kept track of in the config")

[drammock]

so this change makes it so that, when key is None, we get the config value for "MNE_DATA" instead of a big dict of all defined config values.

[drammock]

put another way, it updates the _get_path() function to gracefully handle key=None input, which previously it was not designed to handle.

[larsoner]

Ahh that makes sense, thanks!

[drammock]

Actually you're right, maybe this should still include the fallback of get_config('MNE_DATA') for cases of non-empty strings that aren't valid config keys?

[larsoner]

Or valid ones that aren't set yet in the config, which is pretty common -- people just set MNE_DATA once then all downloads just automatically go to that location

[drammock]

@adam2392 can you review?

[larsoner]

@adam2392 can you review?

@drammock can you force CIs to do some review work for us, too, by creating some new test that fails on main but passes on this PR? Presumably downloading some .mat file like in alphacsc/alphacsc#44 (comment) (I would look at pymatreader for some stable .mat testing file you could use) ? It looks like the only test adjustment is for a dataset version which looks reasonable, but wouldn't have caught the alphacsc error.

Feel free to ignore if this does not make sense or you were going to do this anyway...

[drammock]

can you force CIs to do some review work for us, too, by creating some new test that fails on main but passes on this PR?

done.

[adam2392]

This LGTM.

Note: fatiando/pooch#266 will fix our issues with uncompressing members that are sub-directories.

[drammock]

Note: fatiando/pooch#266 will fix our issues with uncompressing members that are sub-directories.

did you test out fatiando/pooch#266 locally then? You should report back the result on that PR.

[drammock]

CI fails are unrelated:

• test_notebook_alignment and test_notebook_button_counts: DeprecationWarning: Passing a schema to Validator.iter_errors is deprecated and will be removed in a future release. Call validator.evolve(schema=new_schema).iter_errors(...) instead.
• test_3d_functions[mayavi]:
  • ResourceWarning: unclosed context
  • ResourceWarning: unclosed event loop

[larsoner]

Thanks @drammock !

[leouieda]

@drammock we'll be pushing a new release of Pooch today that should fix this bug. Please feel free to reopen the issue linked above if the problem persists.