natverse / nat

NeuroAnatomy Toolbox: An R package for the (3D) visualisation and analysis of biological image data, especially tracings of single neurons.
https://natverse.org/nat/
64 stars 28 forks source link

function for rerooting neuron #463

Closed dokato closed 3 years ago

dokato commented 3 years ago

getting foot in the door.

TODO:

dokato commented 3 years ago

and pointno for both

Here I decided to be more flexible and rather than point belonging to neuron I provide an option to reroot to a node that is closest to point

dokato commented 3 years ago

@jefferis I'd appreciate advice on how to go around this point:

finally we will need a specialised catmaidneuron method that ensures that additional metadata

It was tempting to just use a function from catmaid:

#' @export
reroot.catmaidneuron<-function(object, ...) {
  n = NextMethod()
  copy_tags_connectors(n, object)
}

But this should go to catmaid then? Here it'd introduce a dependency loop.

jefferis commented 3 years ago

Thanks @dokato! Yes, that was exactly what I was expecting the method should look like;reroot.catmaidneuron should be in catmaid. In future we should move some of the support for neurons with connectivity from catmaid into nat. But that will be a medium-sized change, so we should probably plan that separately.

dokato commented 3 years ago

Thanks @jefferis for useful comments, I added support for matrices, node ids and removed pkgdown build. PTAL now.

dokato commented 3 years ago

BTW while on that I gave it a go and added reroot.catmaidneuron to https://github.com/natverse/rcatmaid

But for some reason I'm getting such error:

> conn=catmaid_login(server="https://l1em.catmaid.virtualflybrain.org")
> orns=read.neurons.catmaid("name:ORN right", .progress='text')
> class(orns[[1]])
[1] "catmaidneuron" "neuron"
> reroot.catmaidneuron(orns[[1]])
Error in NextMethod() : generic function not specified

I'm using nat version with the reroot changes ofkz. Maybe I miss something about generics, but I believe that I found some relevant example where the "next" method came from a different package - similar to our case. Any ideas what's wrong here?

jefferis commented 3 years ago

Maybe you need to import the generic from nat?

Sent from my iPhone

On 3 Jun 2021, at 12:15, dokato @.***> wrote:

 BTW while on that I gave it a go and added reroot.catmaidneuron to https://github.com/natverse/rcatmaid

But for some reason I'm getting such error:

conn=catmaid_login(server="https://l1em.catmaid.virtualflybrain.org") orns=read.neurons.catmaid("name:ORN right", .progress='text') class(orns[[1]]) [1] "catmaidneuron" "neuron" reroot.catmaidneuron(orns[[1]]) Error in NextMethod() : generic function not specified I'm using nat version with the reroot changes ofkz. Maybe I miss something about generics, but I believe that I found some relevant example where the "next" method came from a different package - similar to our case. Any ideas what's wrong here?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or unsubscribe.

jefferis commented 3 years ago

Oh also you shouldn’t call the method directly. Use the generic ie plain reroot.

Sent from my iPhone

On 3 Jun 2021, at 12:15, dokato @.***> wrote:

 BTW while on that I gave it a go and added reroot.catmaidneuron to https://github.com/natverse/rcatmaid

But for some reason I'm getting such error:

conn=catmaid_login(server="https://l1em.catmaid.virtualflybrain.org") orns=read.neurons.catmaid("name:ORN right", .progress='text') class(orns[[1]]) [1] "catmaidneuron" "neuron" reroot.catmaidneuron(orns[[1]]) Error in NextMethod() : generic function not specified I'm using nat version with the reroot changes ofkz. Maybe I miss something about generics, but I believe that I found some relevant example where the "next" method came from a different package - similar to our case. Any ideas what's wrong here?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or unsubscribe.

dokato commented 3 years ago

ah damn, of course, that was it :P I'll send PR soon.

jefferis commented 3 years ago

Actually @dokato the generic should have signature (x, ...) and the method should have ... in addition to the named arguments. Generics should pretty much always have ... so that you can handle any subsequent methods that need other arguments.

coveralls commented 3 years ago

Coverage Status

Coverage decreased (-0.06%) to 81.383% when pulling ca00daaf02e7cb6bb663e329ef7eefd2e6142c7a on dokato:reroot into 760f19e2b52b03e286dbb0ba47ba63561c4014c7 on natverse:master.

jefferis commented 3 years ago

@dokato I added some details to the docs. I'll merge when the checks complete.