Skip to content
This repository has been archived by the owner on Mar 15, 2023. It is now read-only.

Reduce the number of warnings in the CI #486

Closed
5 of 6 tasks
h-mayorquin opened this issue Apr 27, 2022 · 2 comments · Fixed by #587
Closed
5 of 6 tasks

Reduce the number of warnings in the CI #486

h-mayorquin opened this issue Apr 27, 2022 · 2 comments · Fixed by #587
Assignees
Labels
low priority Not currently essential for any projects but would be great for a contributor to work on warnings Like a bug, but not as critical

Comments

@h-mayorquin
Copy link
Collaborator

h-mayorquin commented Apr 27, 2022

@bendichter requested me to work to reduce some warnings. In the last two weeks the number has been reduced from around 600 before any warning reduction related PR to around 300 with #481

I think that now most of the low hanging fruit has been taken and the remaining ones have to do with other packages or require modify our tests slightly. So this is to document the work that has been done and establish a roadmap ahead:

The remaining warnings come from different sources. I list them here in no particular order.

As I mentioned, I think that most of the low-hanging fruit has already been picked and I will add this as low-priority unless @bendichter things it should be higher:

  • Deprecationg warning for scipy.io.matlab.mio5_params in roi-extractor.
  • Warning for non description used in test_si (this probably should be solved when we address issue Convert many tests to unittest.TestCases #287
  • warn("could not find TwoPhotonSeries, using ImagingExtractor to create an nwbfile") roiextractors.

As I mentioned, there is also a bunch of errors that come from other libraries. Not clear if they could be solved by updating their version (I think this should be the case for some of the pandas warnings).

Here is a log from running pytest on main on my system for future reference.
warnings_main.txt

@h-mayorquin h-mayorquin added low priority Not currently essential for any projects but would be great for a contributor to work on warnings Like a bug, but not as critical labels Apr 27, 2022
@h-mayorquin h-mayorquin self-assigned this Apr 27, 2022
@h-mayorquin
Copy link
Collaborator Author

h-mayorquin commented Jun 30, 2022

I think we have dealt with the most numerous and egregious warnings. Looking at the current state of affairs it seems that most of the remaining warnings correspond to the movie interface which I will get rid when I fix it to run faster ( see #449). Then, the are some warnings corresponding to missing metadata that should be address after catalystneuro/neuroconv#43.

The only two remaining issues that I think we should settle to close this issue are two pending PRs in spikeextractors:

SpikeInterface/spikeextractors#673
SpikeInterface/spikeextractors#675

Could we merge those so we can close this @alejoe91 @bendichter ?

Then we should be able to close this issue for good.

@h-mayorquin
Copy link
Collaborator Author

We are down from 700 to 100 warnings now. The remaining ones seem to be related to lack of metadata and electrode information which should be fixed once we start to systematically add metadata to our examples. Then there is the MovieInterface.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
low priority Not currently essential for any projects but would be great for a contributor to work on warnings Like a bug, but not as critical
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant