Skip to content

Fix 001712 notebooks#166

Closed
alessandratrapani wants to merge 3 commits into
dandi:masterfrom
catalystneuro:ibl-widefield-notebooks
Closed

Fix 001712 notebooks#166
alessandratrapani wants to merge 3 commits into
dandi:masterfrom
catalystneuro:ibl-widefield-notebooks

Conversation

@alessandratrapani

Copy link
Copy Markdown
Contributor

--> see #142 (comment)

bendichter and others added 3 commits May 13, 2026 22:13
The notebook had `USE_DANDI = False` as the default, which routes
to a hardcoded Windows path `E:/IBL-widefield-nwbfiles/full/` that
only exists on the original author's machine. Colab users (and
anyone else who clicks 'Open in Colab') get:

  ValueError: Expected 1 file, found 0 for pattern sub-FD-28/...

The other two notebooks in the same PR already default `USE_DANDI =
True` — this one was the outlier, probably toggled to False during
local development and not flipped back before submitting.

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

@bendichter

Copy link
Copy Markdown
Member

Closing as superseded — thanks for the work here, @alessandratrapani.

Since this PR was opened (May 14), master gained reworked versions of these notebooks in commit b29f637 "Make 001712 IBL-Widefield notebooks Colab-ready (bootstrap + fixes)" (plus follow-up curl-idempotency commits). The on-master versions:

  • include the Colab-bootstrap install cell that this PR is missing (that missing cell is exactly why CI here fails at the extract stage), and
  • add a 4th notebook (anatomical_localization_widefield.ipynb) not present in this PR.

001712 is not on either exclusion list, so master's versions are actively CI-tested. Merging this PR as-is would regress them (strip the bootstrap cells and re-break CI), so there's nothing to gain by rebasing it.

If any substantive analysis fix you made here didn't make it into the b29f637 rework, please reopen or open a fresh PR on top of current master with just that change and we'll get it in. 🙏

@bendichter bendichter closed this Jun 16, 2026
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