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

Refine query of upstream tables on FigURL populate #871

Merged
merged 4 commits into from
Mar 21, 2024

Conversation

CBroz1
Copy link
Member

@CBroz1 CBroz1 commented Mar 13, 2024

Description

@rpswenson reported issues generating a FigURL with the following key

from uuid import UUID
from spyglass.spikesorting.v1.figurl_curation import FigURLCuration

key = {'sorting_id': UUID('afeb8f45-a8cc-49f6-8609-6c26db8ff273')}
FigURLCuration.populate(key)

This PR addresses this issue by running a join before the fetch of upstream data.

Testing the result however lands on the following error.

Stack
---------------------------------------------------------------------------
AttributeError                            Traceback (most recent call last)
Cell In[1], line 1
----> 1 from spyglass.spikesorting.v1.figurl_curation import FigURLCuration as F; from uuid import UUID; mk={'sorting_id': UUID('afeb8f45-a8cc-49f6-8609-6c26db8ff273')}; F.populate(mk)

File ~/wrk/datajoint-python/datajoint/autopopulate.py:247, in AutoPopulate.populate(self, suppress_errors, return_exception_objects, reserve_jobs, order, limit, max_calls, display_progress, processes, make_kwargs, *restrictions)
    241 if processes == 1:
    242     for key in (
    243         tqdm(keys, desc=self.__class__.__name__)
    244         if display_progress
    245         else keys
    246     ):
--> 247         status = self._populate1(key, jobs, **populate_kwargs)
    248         if status is True:
    249             success_list.append(1)

File ~/wrk/datajoint-python/datajoint/autopopulate.py:314, in AutoPopulate._populate1(self, key, jobs, suppress_errors, return_exception_objects, make_kwargs)
    312 self.__class__._allow_insert = True
    313 try:
--> 314     make(dict(key), **(make_kwargs or {}))
    315 except (KeyboardInterrupt, SystemExit, Exception) as error:
    316     try:

File ~/wrk/spyglass/src/spyglass/spikesorting/v1/figurl_curation.py:155, in FigURLCuration.make(self, key)
    150 unit_metrics = _reformat_metrics(metric_dict)
    152 # TODO: figure out a way to specify the similarity metrics
    153
    154 # Generate the figURL
--> 155 key["url"] = _generate_figurl(
    156     R=recording,
    157     S=sorting,
    158     initial_curation_uri=curation_uri,
    159     recording_label=recording_label,
    160     sorting_label=sorting_label,
    161     unit_metrics=unit_metrics,
    162 )
    164 # INSERT
    165 self.insert1(key, skip_duplicates=True)

File ~/wrk/spyglass/src/spyglass/spikesorting/v1/figurl_curation.py:205, in _generate_figurl(R, S, initial_curation_uri, recording_label, sorting_label, unit_metrics, segment_duration_sec, snippet_ms_before, snippet_ms_after, max_num_snippets_per_segment, channel_neighborhood_size, raster_plot_subsample_max_firing_rate, spike_amplitudes_subsample_max_firing_rate)
    201 sorting = S
    203 sampling_frequency = recording.get_sampling_frequency()
--> 205 this_view = SpikeSortingView.create(
    206     recording=recording,
    207     sorting=sorting,
    208     segment_duration_sec=segment_duration_sec,
    209     snippet_len=(
    210         int(snippet_ms_before * sampling_frequency / 1000),
    211         int(snippet_ms_after * sampling_frequency / 1000),
    212     ),
    213     max_num_snippets_per_segment=max_num_snippets_per_segment,
    214     channel_neighborhood_size=channel_neighborhood_size,
    215 )
    217 # Assemble the views in a layout. Can be replaced with other layouts.
    218 raster_max_fire = raster_plot_subsample_max_firing_rate

File ~/miniconda3/envs/spy/lib/python3.9/site-packages/sortingview/SpikeSortingView/SpikeSortingView.py:40, in SpikeSortingView.create(recording, sorting, segment_duration_sec, snippet_len, max_num_snippets_per_segment, channel_neighborhood_size, bandpass_filter, use_cache)
     29 @staticmethod
     30 def create(*,
     31     recording: si.BaseRecording,
   (...)
     38     use_cache: bool=True
     39 ):
---> 40     data_uri = prepare_spikesortingview_data(
     41         recording=recording,
     42         sorting=sorting,
     43         segment_duration_sec=segment_duration_sec,
     44         snippet_len=snippet_len,
     45         max_num_snippets_per_segment=max_num_snippets_per_segment,
     46         channel_neighborhood_size=channel_neighborhood_size,
     47         bandpass_filter=bandpass_filter,
     48         use_cache=use_cache
     49     )
     50     return SpikeSortingView(data_uri)

File ~/miniconda3/envs/spy/lib/python3.9/site-packages/sortingview/SpikeSortingView/prepare_spikesortingview_data.py:24, in prepare_spikesortingview_data(recording, sorting, segment_duration_sec, snippet_len, max_num_snippets_per_segment, channel_neighborhood_size, bandpass_filter, use_cache)
     13 def prepare_spikesortingview_data(*,
     14     recording: si.BaseRecording,
     15     sorting: si.BaseSorting,
   (...)
     21     use_cache: bool=True
     22 ) -> str:
     23     if use_cache:
---> 24         recording_object = get_recording_object(recording)
     25         sorting_object = get_sorting_object(sorting)
     26         cache_key_obj = {
     27             'type': 'spikesortingview_data',
     28             'version': 2,
   (...)
     34             'channel_neighborhood_size': channel_neighborhood_size
     35         }

File ~/miniconda3/envs/spy/lib/python3.9/site-packages/sortingview/load_extractors/get_recording_object.py:14, in get_recording_object(recording)
     12     return recording.sortingview_object
     13 elif isinstance(recording, se.NwbRecordingExtractor):
---> 14     file_path = recording._file_path
     15     electrical_series_name = recording._electrical_series_name
     16     nwb_file_uri = kcl.store_file_local(file_path, label=os.path.basename(file_path), reference=True) # important to set reference=True

AttributeError: 'NwbRecordingExtractor' object has no attribute '_file_path'

Checklist:

  • This PR should be accompanied by a release: No
  • (If release) I have updated the CITATION.cff
  • I have updated the CHANGELOG.md
  • I have added/edited docs/notebooks to reflect the changes

@CBroz1 CBroz1 requested a review from khl02007 March 13, 2024 22:03
@CBroz1 CBroz1 marked this pull request as ready for review March 13, 2024 22:03
@CBroz1 CBroz1 marked this pull request as draft March 13, 2024 22:05
@CBroz1
Copy link
Member Author

CBroz1 commented Mar 13, 2024

@Kyu - Do you have time to take a look at the error stack in the description? Is this just my version of sortingview?

@Kyu
Copy link

Kyu commented Mar 14, 2024

@CBroz1 unfortunately I think you might've pinged the wrong person. Very cool project though, really interesting stuff!

@CBroz1
Copy link
Member Author

CBroz1 commented Mar 14, 2024

Oops! My bad. Sorry for the confusion. I meant to ping @khl02007

@khl02007
Copy link
Collaborator

@CBroz1 looks good, thanks for fixing this!

khl02007
khl02007 previously approved these changes Mar 14, 2024
@CBroz1 CBroz1 marked this pull request as ready for review March 15, 2024 15:06
@khl02007
Copy link
Collaborator

Actually this may be related to incompatibility between sotringview and spikeinterface. What version of each are you using?

@CBroz1
Copy link
Member Author

CBroz1 commented Mar 15, 2024

Actually this may be related to incompatibility between sotringview and spikeinterface. What version of each are you using?

sortingview                      0.11.15
spikeinterface                   0.99.1

@edeno edeno requested a review from khl02007 March 18, 2024 16:12
@edeno edeno dismissed khl02007’s stale review March 18, 2024 16:13

Resolution of package compatibility issues

@edeno
Copy link
Collaborator

edeno commented Mar 18, 2024

@khl02007 are you suggesting @rpswenson upgrade their spikeinterface version? Should we warn people in the lab that they will have to do this in general for the latest versions?

@khl02007
Copy link
Collaborator

yes, @rpswenson: can you check if upgrading the spikeinterface version that solved yesterday's issue also solves this one?

@CBroz1
Copy link
Member Author

CBroz1 commented Mar 21, 2024

@khl02007 - The issue in the description error stack came from my machine with the target spikeinterface version, so I think this issue will remain

@edeno edeno merged commit b7a2986 into LorenFrankLab:master Mar 21, 2024
7 checks passed
@CBroz1 CBroz1 deleted the figurl_fix branch March 21, 2024 18:52
@samuelbray32
Copy link
Collaborator

Quick note: The end of the spikesorting v1 notebook won't work on the demo until this comes in with a new release.

Can confirm this is not spikesorting version but lack of needed information in the primary key of FigURLCurationSelection

Thanks for fixing!

@rpswenson
Copy link

@khl02007 - The issue in the description error stack came from my machine with the target spikeinterface version, so I think this issue will remain

Quick note: The end of the spikesorting v1 notebook won't work on the demo until this comes in with a new release.

Can confirm this is not spikesorting version but lack of needed information in the primary key of FigURLCurationSelection

Thanks for fixing!

Can also confirm that the figurl populate still doesn't work after fixing the spikeinterface version.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants