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

fixel2voxel split_data: -weighted and -number confusion #1790

Open
thijsdhollander opened this issue Oct 31, 2019 · 1 comment
Labels
bug

Comments

@thijsdhollander
Copy link
Contributor

@thijsdhollander thijsdhollander commented Oct 31, 2019

All relevant info and musings here: https://community.mrtrix.org/t/fibre-orientation-wrt-b0/2850/18

Might be a bug, or might just be the documentation that could benefit from some extra clarification (if not a bug), or both. In terms of documentation, I think what would at least be useful to specify a bit more is:

  • How is split_data output ordered? Is there any sorting that can be relied upon? (doesn't seem like it) Is there any way to obtain some sorting nonetheless? (sounds like a sensible use case)

  • What is "largest" exactly referring to in the documentation of -number?

In terms of improving functionality (even if it's currently not a bug), I'd argue that -weighted does have a potentially meaningful way of applying to split_data, which would effectively be to sort the output. But if that would be offered, I'd recommend to document it very well so users do discover it when they require it. Out of the box, they might still all too easily assume the output of split_data would be sorted by the splitted data's magnitude itself. That itself is something you could discuss or consider even (e.g. as default behaviour?).

Well, all up to you; does look like there's some useful things to consider either way.

@thijsdhollander thijsdhollander added the bug label Oct 31, 2019
@thijsdhollander

This comment has been minimized.

Copy link
Contributor Author

@thijsdhollander thijsdhollander commented Oct 31, 2019

(label might also be "documentation"; but I don't have that option I think)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
1 participant
You can’t perform that action at this time.