Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Cleanup of modules gallery docs (post SorterAnalyzer) #2552

Merged
merged 2 commits into from
Mar 6, 2024

Conversation

zm711
Copy link
Collaborator

@zm711 zm711 commented Mar 6, 2024

@samuelgarcia since you've added zarr into core for SorterAnalyzer we need it to be in base dependencies.... The documentation update broke one of the example code because when you did a zarr with SorterAnalyzer the doc environment didn't have xarray. We could also revert that change and change the example, but at this point it is probably best just to add xarray to core no?

(Also I see for the examples you're using the list input for the SorterAnalyzer.compute()... Nice)

@zm711 zm711 added the documentation Improvements or additions to documentation label Mar 6, 2024
@samuelgarcia
Copy link
Member

xarray is a big stuff for core dep. I think I would prefer avoid it.
It is needed only when using Dataframe and so only quality mertics at the moment. Lets think about this.

pyproject.toml Outdated
@@ -24,6 +24,7 @@ dependencies = [
"threadpoolctl>=3.0.0",
"tqdm",
"zarr>=0.2.16",
"xarray",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Honestly I would prefer to avoid this.
We can add it for doc building.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done. Removed from core. added back to full and to docs.

@zm711
Copy link
Collaborator Author

zm711 commented Mar 6, 2024

But you have it in SorterAnalyzer for zarr format. So it's already in core if someone wants to use zarr format. But for this PR I can switch it back to just be in the docs building.

@samuelgarcia
Copy link
Member

Thanks for the grammar and typos. I can't beleive I did so much mistake.

@zm711
Copy link
Collaborator Author

zm711 commented Mar 6, 2024

It's the curse of someone who types so fast, the examples can't keep up with your mind :) .

@samuelgarcia samuelgarcia merged commit 85619d5 into SpikeInterface:main Mar 6, 2024
11 checks passed
@zm711 zm711 deleted the doc-fixes branch March 6, 2024 19:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants