Skip to content
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

Add 'Electrophysiology' page. #55

Open
wants to merge 62 commits into
base: main
Choose a base branch
from
Open

Add 'Electrophysiology' page. #55

wants to merge 62 commits into from

Conversation

JoeZiminski
Copy link
Member

@JoeZiminski JoeZiminski commented May 15, 2024

As discussed in #53, this PR adds a 'Electrophysiology' page and changes 'Data Analysis' to 'Behaviour' page. The electrophysiology page has added:

  • getting started - quick overview on resources to get started
  • examples - python and matlab code from researchers in the building for analysing ephys data
  • community - some links to various community resources
  • resources - list of interesting reading.

Reviewing:

  • I think the changes to the website-generation machinery are quite minimal, bar adding sphinx-gallery
  • I am interested to know if the text reads okay and example pipeline blurbs are clear. It might easiest to build to read for review.
  • @neuroinformatics-unit/behaviour this rearranges the way the behaviour section is displayed, let me know if you have any feedback / don't like it.

TODO

  • just realised one of the example pipeline repos are private, will follow up.

@JoeZiminski JoeZiminski changed the title Add 'Ephys at the SWC' page. Add 'Electrophysiology at the SWC' page. May 15, 2024
@JoeZiminski JoeZiminski marked this pull request as draft May 15, 2024 11:27
Copy link
Member

@adamltyson adamltyson left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is excellent @JoeZiminski, I think this will be incredibly useful for SWC (and other) researchers. I have some minor comments, and I think the roadmap should be moved out of this website.

docs/source/behav_at_swc/index.md Outdated Show resolved Hide resolved
docs/source/index.md Outdated Show resolved Hide resolved
docs/source/index.md Outdated Show resolved Hide resolved
docs/source/ephys_at_swc/index.md Outdated Show resolved Hide resolved
docs/source/behav_at_swc/index.md Outdated Show resolved Hide resolved
docs/source/ephys_at_swc/getting_started.md Outdated Show resolved Hide resolved
docs/source/ephys_at_swc/getting_started.md Outdated Show resolved Hide resolved
docs/source/ephys_at_swc/getting_started.md Outdated Show resolved Hide resolved
docs/source/ephys_at_swc/roadmap.md Outdated Show resolved Hide resolved
docs/source/ephys_at_swc/community.md Outdated Show resolved Hide resolved
@JoeZiminski
Copy link
Member Author

Hey @adamltyson I've added a very rough version of the example pipelines just to get an idea of the layout. Basically the type of pipeline in use in the building that it would be nice to include roughly falls into three categories:

  1. spikeinterface scripts for preprocessing and sorting (suitable for sphinx-gallery)
  2. full project repos that use python (often spikeinterface) but are fully integrated with multimodal analysis (e.g. Steve's pipeline)
  3. (2) but for MATLAB pipelines (e.g. Mateo's pipeline)

It seems important to include all three categories on the website (e.g. even though we recommend SI there is a lot of valuable methods in those matlab pipelines). But i'm not sure how best to display the information in a non-clunky way. ATM the front page of the sphinx gallery contains preprocessing/sorting scripts in the usual gallery way, but also links to two other pages detailing python and matlab repos. Do you think this works? (and the general approach, giving some backround of named researchers, their setups and studies).

@JoeZiminski
Copy link
Member Author

Ah, now understand that incredibly annoying error! Build was failing with unpinned sphinx with the below error. I was lazily ignoring warnings because it was building fine locally, I think because I had contaminated my env with ipykernel. However CI was failing with the below error:

Notebook error:
NoSuchKernel in ephys_at_swc/gallery/sara_mederos.ipynb:
No such kernel named python3

and was fixed by pinning to sphinx<7.2. The problem was I ignored the following error in latest shpinx:

WARNING: multiple files found for the document "ephys_at_swc/gallery/sara_mederos": ['ephys_at_swc/gallery/sara_mederos.py', 'ephys_at_swc/gallery/sara_mederos.ipynb', 'ephys_at_swc/gallery/sara_mederos.rst']
Use '/Users/joeziminski/git_repos/HowTo/docs/source/ephys_at_swc/gallery/sara_mederos.ipynb' for the build.

so it was trying to use .ipynb for the build by default in later sphinx. In earlier sphinx, it uses the .rst by default:

WARNING: multiple files found for the document "ephys_at_swc/gallery/sara_mederos": ['ephys_at_swc/gallery/sara_mederos.py', 'ephys_at_swc/gallery/sara_mederos.ipynb', 'ephys_at_swc/gallery/sara_mederos.rst']
Use '/Users/joeziminski/git_repos/HowTo/docs/source/ephys_at_swc/gallery/sara_mederos.rst' for the build.

and fixing the warning with the below means it builds okay on most recent sphinx 🎉. I guess a good lesson to always fix warnings first even if they seem unrelated to the error!

exclude_patterns = [
   ...
    "ephys_at_swc/gallery/*.ipynb",
    "ephys_at_swc/gallery/*.py",
]

@adamltyson
Copy link
Member

adamltyson commented May 20, 2024

@JoeZiminski I think the general idea is good, with the obvious caveats that:

  • Everyone is properly credited
  • They're happy with this all being online, and they're aware that it's fully public etc.

@JoeZiminski JoeZiminski marked this pull request as ready for review September 16, 2024 17:00
@JoeZiminski JoeZiminski requested a review from a team September 16, 2024 17:39
@JoeZiminski JoeZiminski changed the title Add 'Electrophysiology at the SWC' page. Add 'Electrophysiology' page. Sep 16, 2024
Copy link
Member

@adamltyson adamltyson left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey @JoeZiminski this is excellent. I've reviewed afresh, sorry if anything I say contradicts my original reviews!

I've left some comments, but I think the main thing to check is conflicts with recent changes to the behavioural guides.

Also, as you mentioned something needs to be done about the private link, either get it made public (ideal) or remove that section for now.

Assuming everyone mentioned is happy for this to be shared, I think this is nearly ready to go!

@@ -27,7 +27,9 @@ jobs:
needs: linting
runs-on: ubuntu-latest
steps:
- uses: neuroinformatics-unit/actions/build_sphinx_docs@v2
- uses: neuroinformatics-unit/actions/build_sphinx_docs@main
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there a reason to use main here? I think in general we want to be using the latest released version (in case a rogue PR goes through). @v2 will resolve to the latest v2.x release.

- uses: neuroinformatics-unit/actions/build_sphinx_docs@v2
- uses: neuroinformatics-unit/actions/build_sphinx_docs@main
with:
python-version: 3.11
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Any chance 3.12 works? Just gives us slightly more runway before it needs changing.

@@ -37,6 +39,6 @@ jobs:
if: github.event_name == 'push' && github.ref_name == 'main'
runs-on: ubuntu-latest
steps:
- uses: neuroinformatics-unit/actions/deploy_sphinx_docs@v2
- uses: neuroinformatics-unit/actions/deploy_sphinx_docs@main
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See above re action version.

Comment on lines -21 to -39
::: {dropdown} Note on managed Linux desktops
:color: info
:icon: info

The SWC's IT team offers managed desktop computers equipped with a Linux image. These machines are already part of SWC's trusted domain and have direct access to SLURM, the HPC modules, and the SWC filesystem.

If you have access to one of these desktops,
you can skip the pre-requisite steps.
You may simply open a terminal, type `module load SLEAP`,
and start using SLEAP directly, as you would on any local
Linux machine. All SLEAP commands should work as expected,
including `sleap-label` for launching the GUI.

That said, you may still want to offload GPU-intensive tasks to an HPC node (e.g. because the desktop's GPU is not powerful enough or because you need to run many jobs in parallel). In that case, you may
still want to read the sections on [model training](sleap-training)
and [inference](sleap-inference).
:::

(access-to-the-hpc-cluster)=
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there a reason why this was removed in this PR?

@@ -53,17 +33,15 @@ $ module avail
...
SLEAP/2023-03-13
SLEAP/2023-08-01
SLEAP/2024-08-14
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This PR seems to be undoing some changes made since it was first raised.

Comment on lines +3 to +6
This site is maintained by the
[NeuroInformatics Unit](https://neuroinformatics.dev/).
Please don't hesitate to contact us
(in particular Joe Ziminski) on Slack with any questions or feedback.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe link to the Zulip here so any external people can get in touch too?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe link each reference to the lab to the official SWC page for that lab? Then there's an easy way to people to find email addresses etc?

striatum. He is using NeuroPixels 2.0 and SpikeGLX with a SpikeInterface-based
pipeline; the code can be found `here <https://github.com/stephenlenzi/npix_lse>`__.

Python Scripts
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's not immediately clear how the scripts section is different to the other parts above.


Sara Mederos (Hofer Lab) performs chronic electrophysiological
recordings from subcortical and cortical areas using 4-shank
Neuropixels 2.0 probes (Open Ephys). Recordings are conducted
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just to make it super obvious what this means to anyone new to the field

Suggested change
Neuropixels 2.0 probes (Open Ephys). Recordings are conducted
Neuropixels 2.0 probes (acquired using [Open Ephys](https://open-ephys.org/)). Recordings are conducted

Mateo Velez-Fort (Margie Lab) investigates the integration of visual
and vestibular information in the visual cortex with the
Margrie lab's 'Translocator' setup. They use
Neuropixels 2.0 (SpikeGLX) for acute recordings from the
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As above, just to make it super clear.

Suggested change
Neuropixels 2.0 (SpikeGLX) for acute recordings from the
Neuropixels 2.0 (acquired using [SpikeGLX](https://github.com/billkarsh/SpikeGLX)) for acute recordings from the

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