Add notebooks for 001712#142
Conversation
|
Check out this pull request on See visual diffs & provide feedback on Jupyter Notebooks. Powered by ReviewNB |
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>
|
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 missingAll three notebooks expect data that isn't present in the published dandiset. Inventory of
Notably absent:
What the notebooks expect
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 Interestingly the Possible explanations
What I'd suggestYou probably know better than me which of the above is right. Two paths forward:
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, |
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>
This PR adds an example notebooks for the 001712 dataset.