Also show measured distances in vx (in addition to nm)

[philippotto]

Todo

• also add to other measurement tools (via right click, for all trees etc etc)
• clean up so that there's no need to measure the tree twice (for nm and for vx)

URL of deployed dev instance (used for testing):

• https://___.webknossos.xyz

Steps to test:

• use the following different measurement "tools":
  • tree-specific menu in trees-tab to measure tree length
  • "measure length of all skeletons" in trees tab
  • shift-rightclick a node and select "path length of this tree"
  • shift-rightclick a node and select "path length to this node"
  • shift-rightclick a node and see the bottom line of the context menu which shows the distance
  • shift-rightclick some position and see the bottom line of the context menu which shows the distance
• in all of the above cases, the distance should be shown in nm and in vx

Issues:

• follow-up to Add measurement tools for trees in tree tab #4898

---

• Updated (unreleased) changelog
• Updated (unreleased) migration guide if applicable
• Updated documentation if applicable
• Adapted wk-connect if datastore API changes
• Needs datastore update after deployment
• Ready for review

[philippotto]

The changes to the api's interface are a bit unfortunate. The probability that someone uses this feature is probably very low and the effort to fix it is quite small. This is why I don't want to bump the api version (which is quite a bit of overhead for us and also for our users which have to check how to upgrade). This is a good example why our current API versioning is sub-ideal :/

Do you agree with this approach, @MichaelBuessemeyer and @daniel-wer? Alternatively, I could keep the old methods and add measureTreeLengthInNmAndVx, measureAllTrees InNmAndVx and measurePathLengthBetweenNodesInNmAndVx. However, that's also quite ugly..

[MichaelBuessemeyer]

How about a third variant:

1. Rename measureTreeLength to measureTreeLengthWithVX or so.
2. And just call this method in measureTreeLength and just return the first value. This should match the interface of the old API right?

You can do the same steps for measurePathLengthBetweenNodes.

[MichaelBuessemeyer]

Nevertheless, I think your solution is fine. You can go ahead and merge this PR if you are against my proposed variant.

Users that would have to adapt their code should be able to handle this small interface change 👍

[philippotto]

How about a third variant:

Rename measureTreeLength to measureTreeLengthWithVX or so.
And just call this method in measureTreeLength and just return the first value. This should match the interface of the old API right?

Yes, that would also work (and is kind of what I meant with my suggestion to have an additional method à la measureTreeLengthInNmAndVx). The one thing which bothers me here is the suboptimal naming of the methods (the old method is limited to nm, but would need to keep its name, and the new method is more powerful but would need to have a more specific name). However, I'm not completely against it. Let's wait for a third opinion :)

[daniel-wer]

Seeing that this method has only existed for a couple of months and its addition wasn't announced, I would also think usage is probably extremely small if not zero. You could include this change in the "breaking" section of the changelog :)

[philippotto]

You could include this change in the "breaking" section of the changelog :)

Good idea! I also agree with your risk assessment so this PR should be good to go now 🕺

[MichaelBuessemeyer]

LGTM except for one small thingy. 🚀 🧊

[MichaelBuessemeyer]

Did you intentionally remove the "." at the end or was this just by accident? I think a dot at the end makes sense as this is a complete sentence :)