natverse / neuprintr

R client utilities for interacting with the neuPrint connectome analysis service
http://natverse.org/neuprintr
3 stars 3 forks source link

simplify handling of prepost/partner specification in connectivity queries #63

Open jefferis opened 4 years ago

jefferis commented 4 years ago

I'd actually be in favor of changing for input/output across all functions -- which will certainly break existing analysis code but should be for the best in the long term. Merge into a dev branch?

Originally posted by @romainFr in https://github.com/natverse/neuprintr/pull/61

IsabelDAlessandro commented 4 years ago

Similar to this concern, I've found the documentation for the output of neuprint_get_synapses() to be somewhat confusing in its description of what prepost=0 and prepost=1 mean.

The current documentation reads: "a data frame, where each entry is a connection between the specified bodyid and its partner, either presynaptic to the bodyid (prepost=0) or postsynaptic (prepost=1).... "

Perhaps the name 'prepost' could be changed to something more descriptive. Alternatively, the documentation could be modified for clarity to read something like:

“Value: A data frame, where each entry is a synaptic connection between the specified bodyid and one partner neuron. Entries with prepost=0 indicate a connection where the partner is postsynaptic to the queried neuron (bodyids in the ‘partners’ column are outputs of the queried neuron). Entries with prepost=1 indicate a connection where the partner is presynaptic to the queried neuron (bodyids in the ‘partners’ column are inputs to the queried neuron)....”

jefferis commented 4 years ago

Thanks @IsabelDAlessandro. As you can see I've never been that keen on this notation myself, but given that I didn't take action sooner, it's not something that we can now remove completely. I will see if we can provide an alternate mechanism but for now I have updated the documentation along the lines that you suggest. Please see ?neuprint_get_synapses or (in a few mins) http://natverse.org/neuprintr/reference/neuprint_get_synapses.html

schlegelp commented 4 years ago

Is there a way to deprecate the column and make a step-wise transition starting with having a additional column with the same data but a more descriptive name or values, and then eventually removing the prepost column altogether?