MAINT: Remove code using vulture confidence 60 #12575
Conversation
larsoner
requested review from
drammock,
agramfort,
dengemann,
wmvanvliet,
adam2392,
mscheltienne,
alexrockhill,
hoechenberger,
jasmainak,
rob-luke,
sappelhoff and
cbrnr
as code owners
|
Wow this is massive |
| def _replace_md5(fname): | ||
| """Replace a file based on MD5sum.""" | ||
| # adapted from sphinx-gallery | ||
| assert fname.endswith(".new") |
There was a problem hiding this comment.
this won't work for Path objects, is that OK?
There was a problem hiding this comment.
Must be, this is just copy-pasted 🙂
|
@drammock this one should be good to go now! |
There was a problem hiding this comment.
startling how much unused code in there. Some of it seems useful enough that I'm wondering if it should either get promoted to public API, or spun off into a utils-like section of mne-incubator. I'm thinking of things like
_is_scaled_mri_subject_copy_channel_ensure_image_in_surface_RAS_pair_grad_sensors_*(for when info absent, e.g., projectors)
Feel free to merge if you disagree; I don't have time to do it myself ATM so I don't want to be a blocker.
| @@ -1007,7 +1007,7 @@ def csd_array_multitaper( | |||
| n_times = len(times) | |||
| n_fft = n_times if n_fft is None else n_fft | |||
|
|
|||
| window_fun, eigvals, mt_adaptive = _compute_mt_params( | |||
| window_fun, eigvals, adaptive = _compute_mt_params( | |||
There was a problem hiding this comment.
yikes, this was probably a bug
Perhaps. I'm happy to leave it out for now and see if people need it and assume YAGNI until then. Plus we don't know the extent to which those snippets of code still work with our codebase at this point since they haven't been tested. It's easy enough to find in the history especially knowing the lines were removed in this |
Turns out we had a bunch of dead code. We also had a bunch of unused global vars in our test files which this caught!
This also makes me think that we should maybe add a test that checks the inventory of the testing dataset to make sure all files are used. Given the removals here it's possible that some no longer are, and it's either a bug and we should add them back somehow or we should remove them from the testing dataset...