Dandi export and read #956

samuelbray32 · 2024-05-07T19:13:56Z

Description

Partial steps for #861

Step (3)
- makes new table DandiPath
  - tracks translations between analysis file name and renamed path in a dandiset
  - has method to stream data from dandi
- makes function Export.compile_dandiset()
  - Uses dandi api functions to get dandi-compliant names for files
  - organizes and uploads files to dandi
  - populates DandiPath
- makes admin function Export._prepare_files_for_export()
  - Addresses common discrepancies between dandi standards and metadata in the nwb
  - ensures all analysis files in the export set have unique uuid's Uniqueness of object_ids in generated analysis nwb files #953
  - resolves resulting mismatch between database checksum and altered file
Step (4)
- adds new fallback to fetch_nwb which streams file from dandi if available

Checklist:

This PR should be accompanied by a release: unsure
If release, I have updated the CITATION.cff
This PR makes edits to table definitions: no
N/A If table edits, I have included an alter snippet for release notes.
I have updated the CHANGELOG.md with PR number and description.
I have added/edited docs/notebooks to reflect the changes
Change the dandi url's from development to production server

edeno · 2024-05-08T00:18:32Z

I know this is still a WIP, but we should also decide whether to make dandi a dependency or an optional dependency.

CBroz1 · 2024-05-08T14:40:06Z

src/spyglass/common/common_dandi.py

+    definition = """
+    -> Export.File
+    ---
+    dandiset_id: str


Is the dandiset_id the same for all files in a given Export? If so, I might avoid some redundancy by having DandiPath be a master table with one part per Export.File

As written, the dandiset ID is common for everything in export. However I could see a future case where an analysis file is used in multiple papers and rather than re-uploading a new copy of the file to dandi you would point to it's existing version in a different dandiset. I might lean towards leaving the flexibility for this in the table structure

How does dandi handle this case? Presumably, they wouldn't want to store two copies of the same data file across dandisets. But, if the name is contextually determined by other files, the name would have to differ. I wouldn't be surprised if they had cross-dandiset soft-link tool, but I don't know for sure

It would have different names on Dandi, but they would share the same object_id for the file. I haven't tested if that blocks upload.

CBroz1

Just took a quick first pass to keep up to speed

src/spyglass/common/common_usage.py

src/spyglass/common/common_dandi.py

src/spyglass/sharing/sharing_kachery.py

src/spyglass/utils/nwb_helper_fn.py

review-notebook-app · 2024-05-20T21:01:39Z

Check out this pull request on

See visual diffs & provide feedback on Jupyter Notebooks.

Powered by ReviewNB

into dandi_export

samuelbray32 · 2024-05-20T21:19:57Z

I'm leaving the functions pointing to the Dandi dev server for now until the other parts of the code are reviewed.

CBroz1

Good work, @samuelbray32. I had some comments, but I think the heaviest lifting would be migrating it all to common_dandi to revise the dependency order. I'm happy to help migrating to a more object-oriented approach so the helpers can share properties

notebooks/py_scripts/05_Export.py

CBroz1 · 2024-05-21T14:13:42Z

notebooks/py_scripts/05_Export.py

+#    >
+#    >To aid in correcting common formatting errors identified with changes in dandi standards, we have included the method
+#    ```
+#    Export()._prepare_files_for_export(paper_key)


If it's a func we think users will adopt, I'd remove the _ prefix that implies 'this is just for other funcs to call'

I was using this to try to indicate it's function that shouldn't be used frequently since it requires admin permissions. Open to other ideas of how to indicate this or can remove

notebooks/py_scripts/05_Export.py

src/spyglass/common/common_dandi.py

CBroz1 · 2024-05-21T15:56:02Z

src/spyglass/common/common_usage.py

+        # make a temp dir with symbolic links to the export files
+        source_files = (self.File() & key).fetch("file_path")
+        paper_dir = f"{export_dir}/{paper_id}"
+        os.makedirs(paper_dir, exist_ok=True)


If replacing os with pathlib, you can pathlib_dir.mkdir(parents=True, exist_ok=True)

src/spyglass/common/common_usage.py

CBroz1 · 2024-05-21T16:02:51Z

src/spyglass/common/common_usage.py

+        translations = [
+            {
+                **(
+                    self.File() & key & f"file_path LIKE '%{t['filename']}'"


My file_like helper would work here i think

samuelbray32 · 2024-05-21T17:14:28Z

Thanks @CBroz1. I can work on moving the functionality into dandi_common. For another common comment, dandi is already a dependency in pynwb so it shouldn't cause issues for import

rly · 2024-05-22T20:44:42Z

FYI dandi is not a dependency of pynwb.

samuelbray32 added 10 commits May 3, 2024 12:30

compile exported files, download dandiset, and organize

8808041

add function to translate files into dandi-compatible names

f57287a

add table to store dandi name translation and steps to populate

717e04f

add dandiset validation

675d929

add function to fetch nwb from dandi

1e7491b

add function to change obj_id of nwb_file

58d6411

add dandi upload call and fix circular import

8aa9242

debug dandi file streaming

463a1d6

fix circular import

ff7b5c6

resolve dandi-streamed files with fetch_nwb

204859d

CBroz1 reviewed May 8, 2024

View reviewed changes

samuelbray32 added 7 commits May 20, 2024 09:02

Merge remote-tracking branch 'origin/master' into dandi_export

b641642

implement review comments

94ae801

add admin tools to fix common dandi discrepencies

49c493b

implement tool to cleanup common dandi errors

9cb11a3

add dandi export to tutorial

6a7e486

fix linting

530b128

update changelog

c8f4200

samuelbray32 added 3 commits May 20, 2024 14:08

Merge branch 'master' into dandi_export

00d1364

fix spelling

73a3f97

Merge branch 'dandi_export' of https://github.com/LorenFrankLab/spyglass

aba687c

into dandi_export

samuelbray32 marked this pull request as ready for review May 20, 2024 21:17

edeno requested a review from CBroz1 May 20, 2024 21:18

CBroz1 requested changes May 21, 2024

View reviewed changes

samuelbray32 added 10 commits May 29, 2024 11:03

style changes from review

d9bf04e

reorganize function locations

21bbd3f

fix circular import

9fbb80c

make dandi dependency optional in imports

45f8d3e

store dandi instance of data in DandiPath

dba04df

resolve case of pre-existing dandi entries for export

f716ea4

cleanup bugs from refactor

5727528

update notebook

252f5f4

Merge remote-tracking branch 'origin/master' into dandi_export

4acf2e8

Merge branch 'master' into dandi_export

c693a93

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Dandi export and read #956

Dandi export and read #956

samuelbray32 commented May 7, 2024 •

edited

edeno commented May 8, 2024

CBroz1 May 8, 2024

samuelbray32 May 20, 2024

CBroz1 May 21, 2024

samuelbray32 May 21, 2024

CBroz1 left a comment

review-notebook-app bot commented May 20, 2024

samuelbray32 commented May 20, 2024

CBroz1 left a comment

CBroz1 May 21, 2024

samuelbray32 May 29, 2024

CBroz1 May 21, 2024

CBroz1 May 21, 2024

samuelbray32 commented May 21, 2024

rly commented May 22, 2024

Dandi export and read #956

Are you sure you want to change the base?

Dandi export and read #956

Conversation

samuelbray32 commented May 7, 2024 • edited

Description

Checklist:

edeno commented May 8, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

CBroz1 left a comment

Choose a reason for hiding this comment

review-notebook-app bot commented May 20, 2024

samuelbray32 commented May 20, 2024

CBroz1 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

samuelbray32 commented May 21, 2024

rly commented May 22, 2024

samuelbray32 commented May 7, 2024 •

edited