-
Notifications
You must be signed in to change notification settings - Fork 34
Documentation: Update intro tutorial #749
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
- Removed unused / commented out code - Removed stale code - Refactored blocks into separate local functions - Updated dependent html scripts
Ensure generated classes are from latest schema version when running each tutorial
Fix issue where NWB types are added in the base workspace when running livescripts, creating potential schema version conflicts if exporting multiple livescripts in one go that uses different schema versions
Added function disclaimer / warning
Fixed typo
Allow scrolling in code blocks Add javascript to keep the button pinned to upper right corner
Removed unnecessary listener
Remove use of interp, as it requires Signal Processing Toolbox
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #749 +/- ##
=======================================
Coverage 95.59% 95.59%
=======================================
Files 184 184
Lines 6443 6443
=======================================
Hits 6159 6159
Misses 284 284 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
tutorials/private/mcode/intro.m
Outdated
| % |*TimeSeries*|> object. | ||
|
|
||
| % Synthetic 2-D trajectory (helper returns a 2×300 array) | ||
| data = matnwb.tutorial.intro.getRandomTrajectory(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd rather not create functions for this. I think it might be hard for people to understand what is going on
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, maybe you are right. But using random vectors on the other hand feels rushed and generic.
tutorials/private/mcode/intro.m
Outdated
|
|
||
| trials = types.core.TimeIntervals( ... | ||
| 'colnames', {'start_time', 'stop_time', 'correct'}, ... | ||
| 'colnames', {'start_time', 'stop_time', 'time_to_find'}, ... |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why change the column here? I thought it was useful to show how to store boolean values
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wanted to make the tutorial more cohesive by following a simple imaginary experiment throughout. The current version feels a bit disjoint.
If showing logical values is useful, it should be demonstrated more explicitly. I did not realise that showing the use of booleans was a goal here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
could we add a boolean value back to demonstrate adding different data types?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
could we add a boolean value back to demonstrate adding different data types?
Fix #672
Depends on
Documentation: Add copy button to code blocks #730Motivation
Simplify introduction tutorial and make it follow a story line instead of demonstrating the use of types in a more disconnected manner.
PR Changes
How to test the behavior?
Checklist
fix #XXwhereXXis the issue number?