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

H01 interface #162

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

H01 interface #162

wants to merge 2 commits into from

Conversation

jinhan
Copy link

@jinhan jinhan commented Sep 11, 2024

H01 interface based on the latest master branch

  • add navis/interfaces/h01.py
  • add docs/examples/3_interfaces/plot_04_interface_h01.py

@schlegelp
Copy link
Collaborator

schlegelp commented Sep 12, 2024

Thanks for this PR. I left a few comments above. In particular the docstrings need a bit of work as they still contain references to the MICrONS dataset.

Additionally I have two questions/suggestions:

  1. You seem to be using your own CAVE service (global.brain-wire-test.org) and I was wondering whether users had to keep that in mind if they wanted to store their token manually? I'm guessing it changes the name of the secrets file from global.daf-apis.com-cave-secret.json to global.brain-wire-test.org-cave-secret.json? Perhaps you could add a note to the tutorial. I'd use an admonition - something like this:
    !!! note "Saving your token manually"
        If you want to save your token manually, you have to ...
  2. There is quite a bit of shared functionality now between the H01 and the MICrONS interface. If you have the appetite for it, it would be nice to refactor a little to reduce code duplication. For example, we could minimally put the _chunks_to_nm and _fetch_single_neuron functions into a cave_utils.py module and re-use them in both h01.py and microns.py:
     └── interfaces
         ├── h01.py 
         ├── microns.py
         └── cave_utils.py  <- put shared functions here
    Most of the other functions could also be put into cave_utils and just thinly wrapped in h01.py. For example:
     def fetch_neurons(
         x,
         *,
         lod=2,
         with_synapses=True,
         datastack="h01_c3_flat",
         parallel=True,
         max_threads=4,
         **kwargs,
     ):
         """Fetch neuron meshes.
         ...
         """
         return cave_utils._fetch_neurons(
             x,
             lod=lod,
             with_synapses=with_synapses,
             datastack=datastack,
             parallel=parallel,
             max_threads=max_threads,
             **kwargs,
         )

None of the above are mission critical and I'm happy to take a stab myself later.

As an aside: unfortunately, the tutorial tests are currently only run on push but not on pull_request. That's my fault - I will fix that once this PR is merged.

@jinhan
Copy link
Author

jinhan commented Sep 17, 2024

Thank you for the comments.

  1. I also found that the H01 interface doesn't need the Auth class. With the H01's server_address, I can create a CAVE client. In addition, I checked that the token is saved in global.brain-wire-test.org-cave-secret.json under the .cloudvolume/secrets folder.

  2. I refactored h01.py and microns.py by introducing cave_utils.py, as you suggested.

@schlegelp
Copy link
Collaborator

Sorry, I haven't had the time to properly look through the PR yet. Will try to do that in the second half of next week!

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