Skip to content

Add notebooks for 001712#142

Merged
bendichter merged 7 commits into
dandi:masterfrom
catalystneuro:ibl-widefield-notebooks
May 14, 2026
Merged

Add notebooks for 001712#142
bendichter merged 7 commits into
dandi:masterfrom
catalystneuro:ibl-widefield-notebooks

Conversation

@alessandratrapani
Copy link
Copy Markdown
Contributor

This PR adds an example notebooks for the 001712 dataset.

@review-notebook-app
Copy link
Copy Markdown

Check out this pull request on  ReviewNB

See visual diffs & provide feedback on Jupyter Notebooks.


Powered by ReviewNB

alessandratrapani and others added 6 commits April 23, 2026 16:23
Brings the PR branch up to date with recent merges (codespell fix,
Colab-ready notebooks via PRs dandi#100/dandi#150/dandi#151/dandi#152, etc).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Fixes the CI failures on PR dandi#142:

1. `load_nwb_utils.py` called `client.dandi_authenticate()` —
   001712 is now `embargo_status: OPEN`, so removed it (Colab users
   don't have DANDI creds wired up either way).

2. All three notebooks had `from .load_nwb_utils import *`. The
   leading dot is a relative import that only resolves when the
   notebook is inside a package — outside one (Colab, plain Jupyter)
   it raises ImportError. Dropped the dot.

3. Added the standard 4-cell Colab bootstrap to each notebook:
   - badge
   - 'Installing requirements' markdown
   - collapsed install cell with fully pinned deps + helper-fetch curl
   - restart-runtime admonition

   Locks generated via `uv pip compile` with Colab's preinstalled
   versions as constraints (.github/colab-preinstalled.txt). Key
   pins match Colab exactly on numpy (2.0.2), pandas (2.2.2),
   matplotlib (3.10.0), h5py (3.16.0), scikit-image (0.25.2), etc.
   pynwb pinned to 3.1.3 (required by ndx-anatomical-localization).

4. Dropped `environment.yml` — the pinned install cell is the
   single source of truth, and the conda env was specifying deps
   (ibllib, plotly) the notebooks don't actually use.

5. Simplified README to point users at Colab directly instead of
   the multi-step conda-env-create instructions.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
CI failure on PR dandi#142 traced to: the bootstrap's
`!curl -o load_nwb_utils.py https://.../master/.../load_nwb_utils.py`
overwrites the local file with the master version. But 001712 isn't
on master yet, so curl saves '404: Not Found' as the file content;
Python then errors at import with 'SyntaxError: illegal target for
annotation' on the first line.

Two-part fix to the curl in each of the 3 notebooks:

  !curl --create-dirs -sL -o file URL

becomes

  !test -f file || curl --create-dirs -sLf -o file URL

- `test -f file ||` skips the curl entirely when the file is
  already present (CI: file is in the checkout; idempotent re-runs).
- `-f` (--fail) makes curl exit non-zero on HTTP errors instead of
  writing the error page as the file — so a future 404 would surface
  as a clean helper-stage failure with a real error message.

This also makes the bootstrap robust to any unmerged-helper-file
scenario (CI on a PR that adds a helper file, before that PR merges).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Reverts 73dc267. Per request: it's fine if the curl only works
post-merge — the bootstrap pattern stays consistent with the rest
of the repo's helper-fetch lines.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@bendichter bendichter merged commit e034471 into dandi:master May 14, 2026
3 checks passed
@bendichter
Copy link
Copy Markdown
Member

Tested the merged branch and hit a data-side problem that I think needs to be resolved before this PR can run end-to-end. Surfacing here so you can decide how to proceed.

What's missing

All three notebooks expect data that isn't present in the published dandiset.

Inventory of 001712/draft: only 2 subjects, each with one desc-raw NWB file:

Path Size
sub-FD-24/sub-FD-24_ses-..._desc-raw_behavior+ophys.nwb (single raw file)
sub-FD-28/sub-FD-28_ses-81f90b18-..._desc-raw_behavior+ophys.nwb 33 GB

Notably absent:

  • No subject CSK-im-011 (used as default in processed_widefield.ipynb).
  • No desc-processed (or any other description besides raw) for any subject.

What the notebooks expect

anatomical_localization_widefield.ipynb does:

atlas_registration = nwbfile.lab_meta_data["atlas_registration"]   # KeyError
# … later …
localization = nwbfile.lab_meta_data["localization"]                # also missing
aci = localization.anatomical_coordinates_images["AnatomicalCoordinatesImageIBLBregma"]

I streamed the FD-28 file and inspected its actual structure: there's no lab_meta_data group at all. Top-level groups under /general/ are:

devices, ibl_metadata, institution, lab, optophysiology, protocol,
session_id, source_script, subject

Interestingly the ndx-anatomical-localization namespace is registered in the file (at /specifications/ndx-anatomical-localization/0.1.0/), but no instance of any of its types actually exists in the data. So the schema's there but never got populated.

Possible explanations

  1. The notebooks were written against a dev version of the data that has these objects, and what's on DANDI is an earlier raw-only export.
  2. The atlas-registration/localization step was supposed to be a separate upload (maybe a desc-processed file), which hasn't happened yet.
  3. Two separate dandisets were merged conceptually and only one half made it to publication.

What I'd suggest

You probably know better than me which of the above is right. Two paths forward:

  • Re-upload data with the atlas-localization NDX instances populated (and the CSK-im-011 / desc-processed files if those were also intended). The notebooks should then work as-is.
  • Trim the notebooks to only demonstrate what's actually in the published files (subject/session loading, behavior + ophys access, etc.) and drop the atlas-localization sections — open a follow-up PR with the upload + re-add the atlas notebooks then.

Either way, this isn't a notebook-content bug I can patch — the data the notebook reads doesn't exist in the published dandiset yet. Let me know which direction you'd like to go and I can help.

For reference, the previous commits I pushed onto this branch (Colab bootstrap, dandi_authenticate removal, USE_DANDI default flip, relative-import fix) are still valid — they get the notebooks past the install + load stages before they hit this data-shape issue.

alessandratrapani pushed a commit to catalystneuro/example-notebooks that referenced this pull request May 14, 2026
The 001712/IBL-Widefield notebook (PR dandi#142) uses `aci` as a local
variable shorthand for the NWB field `anatomical_coordinates_image`:

  aci = localization.anatomical_coordinates_images['AnatomicalCoordinatesImageCCFv3']
  space = aci.space
  x_coords = aci.x[:]
  ...

Codespell flagged every `aci` token as a typo for `acpi` —
~50 errors across one PR, blocking CI.

Adding `aci` to ignore-words-list (same pattern as the `makin`
fix in dandi#154 for the CEBRA author surname).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
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.

2 participants