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

Fixes Apptainer bug plus small fixes #2053

Merged
merged 11 commits into from
Oct 2, 2023
Merged

Fixes Apptainer bug plus small fixes #2053

merged 11 commits into from
Oct 2, 2023

Conversation

mhhennig
Copy link
Member

Main change: change to pip install --user for sorters run in containers so python packages are installed in writable location. This otherwise breaks apptainer.

There are a few small changes, sorry they all end up in the same pull request. There were some type issues in the mda and numpy extractors.

mhhennig and others added 9 commits June 29, 2023 12:44
…main

Conflicts:
	src/spikeinterface/core/base.py
	src/spikeinterface/core/numpyextractors.py
Apptainer fails to pip install into the system directory (not writable by default, no space when writable), and the --user flag ensures packages are installed in a writable location. Note not tested with docker.
@alejoe91 alejoe91 added the container Issues related to container (docker/singularity) versions of sorters label Sep 29, 2023
@alejoe91
Copy link
Member

Thanks @mhhennig !!

Yeah no worries about the small changes this time! They are smalle enough :)

@samuelgarcia
Copy link
Member

Hi Matthias.
What are the issue with numpy extractors ?

@alejoe91
Copy link
Member

Hi Matthias. What are the issue with numpy extractors ?

Seems only MdaSorting

@mhhennig
Copy link
Member Author

Hi Matthias. What are the issue with numpy extractors ?

Seems only MdaSorting

NumpyExtractor: assert times.dtype.kind == 'i', 'numpy array of spike times must be integer'
We must check for any integer type (failed for me for a sorting I had), so needs to be:
assert (times.dtype.kind == 'i') or (times.dtype.kind == 'u'), 'numpy array of spike times must be integer'

@alejoe91
Copy link
Member

alejoe91 commented Oct 2, 2023

@mhhennig to keep the repo clean, we all make PRs from forks, and not internal branches. Could you also do it next time? ;)

@alejoe91
Copy link
Member

alejoe91 commented Oct 2, 2023

@mhhennig to keep the repo clean, we all make PRs from forks, and not internal branches. Could you also do it next time? ;)

This is what I mean:
image

@alejoe91
Copy link
Member

alejoe91 commented Oct 2, 2023

Hi Matthias. What are the issue with numpy extractors ?

Seems only MdaSorting

NumpyExtractor: assert times.dtype.kind == 'i', 'numpy array of spike times must be integer' We must check for any integer type (failed for me for a sorting I had), so needs to be: assert (times.dtype.kind == 'i') or (times.dtype.kind == 'u'), 'numpy array of spike times must be integer'

I don't see that assertion, can you share the link?

@alejoe91 alejoe91 merged commit ca48f6b into main Oct 2, 2023
8 checks passed
@alejoe91 alejoe91 deleted the matthias branch October 2, 2023 16:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
container Issues related to container (docker/singularity) versions of sorters
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants