navis-org / navis

Python library for analysis of neuroanatomical data.
https://navis-org.github.io/navis/
GNU General Public License v3.0
83 stars 33 forks source link

allow me not to remove duplicate vertices from cloudvolume navis #167

Closed fcollman closed 1 month ago

fcollman commented 1 month ago

Is your feature request related to a problem? Please describe. I want to do analytics on mesh graphs, and so removing duplicate vertices is problematic around self contact locations where meshes can share vertex locations but we don't want the mesh graph connected.

here's an example https://spelunker.cave-explorer.org/#!middleauth+https://global.daf-apis.com/nglstate/api/v1/6562730992467968 where you can see this axon is making a self contact onto the soma (actually an autapse) but the mesh is not connected, and the l2graph is not connected, so the mesh vertices are correctly not de-duplicated at the contact locations, although they do share voxel positions often.

Describe the solution you'd like I would like the process portion of trimesh that de-duplicates features to be optional when patching cloud-volume. Removing NaNs and infinite values is fine, but de-duplication is not. We've done some extensive work in cloud-volume to make sure that vertice de-duplication is done properly at mesh chunk seems without causing self contacts to corrupt the topology of the mesh graph, and so I'd like to be able to keep that work in navis.

schlegelp commented 1 month ago

Yes, fair point. My first intuition is to just not validate/process the cloud-volume mesh. NaNs and infinite values should not be an issue with those meshes and if users want to de-duplicate, they can call MeshNeuron.validate() explicitly.

Changing this line in utils/cv.py to

n = core.MeshNeuron(v, id=k, units='nm', validate=False, process=False)

should do the trick. I saw you forking the repo - would you like to do a PR for this?

fcollman commented 1 month ago

yes. i've got the basics in place, was playing around with my fork/branch to test it first. https://github.com/fcollman/navis/tree/process%3DFalse-option

this made the default "true" popping the process kwargs off the stack like the return_navis does now.. easy to flip it to default=False.

schlegelp commented 1 month ago

Cool! I think setting the default to False is the better choice?