Move overview #227
Move overview #227
Conversation
BalzaniEdoardo
requested review from
gviejo and
billbrod
and removed request for
gviejo
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## development #227 +/- ##
===============================================
- Coverage 97.30% 97.24% -0.06%
===============================================
Files 18 20 +2
Lines 1669 1815 +146
===============================================
+ Hits 1624 1765 +141
- Misses 45 50 +5 ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
In general, I think it's worth getting Alex's and Eero's opinion on this bit.
- I think the first part doesn't flow into the next. Maybe make it simpler? It feels weird to spend all this text describing non-linear mapping vs. convolution and then barely touch on it in the rest of the text. Also, the first section has a fairly substantial amount of prose, whereas the rest is almost all code. Maybe remove everything from the paragraph that ends "position or spike counts." to the one that starts "On the other hand, the
glmmodule..."
docs/quickstart.md
Outdated
|
|
||
| ###### Simulate Spike Counts | ||
|
|
||
| Let's generate some spikes from a Poisson model. |
There was a problem hiding this comment.
should point out that most users won't do this -- they'll be getting this data from their experiment.
makes me wonder if we should show this with simulated data (vs. loading in a small example dataset)
There was a problem hiding this comment.
for a quickstart I think it is fine to use fake data. I would not use an example data here because explaining it takes more time than actually drawing some samples.
What we should aim for is executable code, the input is not relevant, so I would not go into the load some data route.
There was a problem hiding this comment.
I removed the section and added a simulation of the counts in the code; this should give less emphasis to this step
Co-authored-by: William F. Broderick <[email protected]>
Co-authored-by: William F. Broderick <[email protected]>
I don't know if I would remove that, it is very important to understand the basis object. We may refer to this modes of operation in the subsequent examples more explicitly to improve the flow of the description |
docs/quickstart.md
Outdated
|
|
||
| # compute log-likelihood | ||
| ll = glm.score(X, spike_counts) | ||
| ``` | ||
|
|
||
| ### Model Arguments |
There was a problem hiding this comment.
I would stop here the Quickstart maybe. To me this is more part of the API.
There was a problem hiding this comment.
I removed this section but I added a couple of things:
- Continuous observation: showing how to set up a gamma glm
- Regularization
docs/quickstart.md
Outdated
| observation_model=nmo.observation_models.GammaObservations() | ||
| ) | ||
| ``` | ||
|
|
||
|
|
||
| ### Pre-processing with `pynapple` |
There was a problem hiding this comment.
... and I would keep the remaining in the Quickstart but maybe with an extra title like "Compatibility with other packages". But not sure. This could be a page on its own but not sure where it would fit.
Co-authored-by: William F. Broderick <[email protected]>
Co-authored-by: William F. Broderick <[email protected]>
Co-authored-by: William F. Broderick <[email protected]>
Co-authored-by: William F. Broderick <[email protected]>
Co-authored-by: William F. Broderick <[email protected]>
|
Addressed all concerns |
In this PR I am streamlining the home page content by