-
Notifications
You must be signed in to change notification settings - Fork 9
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
Updates installation instructions and CONTRIBUTING #230
Conversation
more about jupyter, optional deps
more info about how branching and contribution workflow works
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #230 +/- ##
=======================================
Coverage 86.84% 86.84%
=======================================
Files 35 35
Lines 3344 3344
=======================================
Hits 2904 2904
Misses 440 440 ☔ View full report in Codecov by Sentry. |
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.
Overall looks great, not many comments.
CONTRIBUTING.md
Outdated
[contributing to the code below](#contributing-to-the-code) for more details on | ||
this process. | ||
|
||
The amount and form of documentation to add depends on the size of the submitted |
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 got a little lost here maybe change "to add depends on.." to "that would need to be added along with your changes would depend on.." (in order to pace the phrasing in the previous para). On the other hand, this is providing quite a bit of low-level detail that might be better lifted and shifted to a different section of this document. I'm not sure it belongs 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.
Much improved!
docs/install.rst
Outdated
Running the notebooks | ||
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ | ||
|
||
Once you have jupyter installed and the kernel set up, navigate to plenoptic's ``examples/`` directory on your terminal and activate the environment you installed jupyter into (``plenoptic`` for 1, ``base`` for 2 or 3), then run ``jupyter`` and open up the notebooks. If you followed the second or third method, you should be prompted to select your kernel the first time you open a notebook: select the one named "plenoptic". |
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 not just go to the dir, conda activate plenoptic
, and get started?
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.
you have to activate the environment that jupyter is in, so you'll run conda activate plenoptic
only if you followed method 1. does that make sense?
Adds some info to help explain optional dependencies and the branching workflow (@EricThomson I based a lot of that on what you have in Caiman, which is great!)