Added tutorials to documentation #173
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #173 +/- ##
==========================================
- Coverage 86.60% 85.45% -1.16%
==========================================
Files 32 33 +1
Lines 1038 1100 +62
==========================================
+ Hits 899 940 +41
- Misses 139 160 +21
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
fix: swap from info to tips fix: add to index fix: add to index
CodyCBakerPhD
force-pushed
the
add_tutorial
branch
from
bbfddae to
055005b
Compare
feat: saving state feat: saving state
CodyCBakerPhD
added
documentation
asmacdo
left a comment
asmacdo
left a comment
There was a problem hiding this comment.
CLI works, but the python library needs love.
One of the benefits of using sphinx and rst is that we could potentially put these blocks in files, render them in these code blocks with literalinclude, and also execute those files in the tests (or sphinx.ext.doctest, but I havent used that).
Good idea, I'll set that up |
|
@candleindark Ready for review |
by now, at least I, already forgot what we talked about ;-) was that zoom session recorded? I only remember that I recommended, as part of sanitization to remove *remove common prefix from labels for multiple files provided since files names should tell what is different, and not really encode metadata, see Filesystem structure & Filenames richness versus distinctness" but for that we need to operate with multiple files to start with and/or have it "discovered" and then recorded for the dandiset to follow. Altogether also would be good to point to BIDS: Common Principles: Filesystem structure |
docs/tutorials.rst
Outdated
|
|
||
| .. code-block:: bash | ||
|
|
||
| cd ~/.nwb2bids/tutorials/ephys_tutorial_file |
There was a problem hiding this comment.
it might be nice to inlcude the tutorial command to populate each of these. For those who dont mind the hardcoded path in home, these cds make perfect sense, but I used -o so i could put this in /tmp--, and I kept using ephys_tutorial_dataset. Obviously user error, but just wanted you to be aware of that hole to step in :)
There was a problem hiding this comment.
As per Yariks suggestion these have been moved to the top as pre-commands
After thinking on having only one command vs. two (separate for single-file vs. dataset) I think it is nice to have different scopes (and different tutorials) that are able to focus on different aspects (more detailed file contents vs. broader dataset-level effects)
Those sound like the links we want, thanks |
Co-authored-by: Austin Macdonald <[email protected]>
|
Added note with links about naming practices, separated generation sections as preambles, and debugged the issue with part 4 and 6 |
|
@asmacdo Ready for eyes again A quick way to verify output is produced is to just run |
yarikoptic
left a comment
yarikoptic
left a comment
There was a problem hiding this comment.
but let's still move generation of nwb files into Prerequisites section which would cover installation and that data generation
|
@asmacdo Drat, you also need to approve since you had previously requested blocking changes 😆 |
Main addition is docs, but relies on some minor utility functions in codebase + CLI invocation