Coordinate transforms are a hassle

openworm / tracker-commons

Compilation of information and code bases related to open-source trackers for C. elegans

11 stars 12 forks source link

Coordinate transforms are a hassle #134

Open Ichoran opened 7 years ago

Ichoran commented 7 years ago

The "centroid can act as the origin" scheme I proposed turns out to considerably complicate the implementations.

I propose that we do away with the dual function of centroid:

The origin is the only origin. All coordinates are relative to it, if it is present. It is (0, 0) if not present.
The centroid is kept track of if present, but it doesn't do anything.

MichaelCurrie commented 7 years ago

Given that we've already implemented this feature, are we sure it's a good idea to go back and change it now? Is it causing active problems for you?

Thanks

Ichoran commented 7 years ago

I was rewriting for perimeters and noticing that this feature made it a considerably trickier rewrite than if it wasn't there.

It's not a fatal flaw. It's just an extra burden on other people who implement this which is maybe not called for.

But the "leave it alone already" sentiment is compelling too, which is why I wanted to raise the issue.

Ichoran commented 7 years ago

@MichaelCurrie - The problem with origins as they stand now is that there is no way to avoid the feature without getting gobbledygook. You absolutely must parse the correct origin in order for the spine positions to make any sense. So it's absolutely a required feature for a reader (not a writer) to properly interpret the numbers.

That's why I think it's worth the effort to make the change.

If there is an origin "ox", "oy", all other values are relative to that. If not, all other values are absolute. This way readers don't even have to care whether cx and cy exist.

Again, I don't like making a change like this so late, but I really think simplicity is to be strived for with the basic feature set. Doing without an origin can dramatically increase file sizes, though, so I don't think we can mandate that everything be global (which would be even simpler).

Ichoran commented 7 years ago

The simplest version (ox/oy are either the same dimension as t, or are absent) is now documented in the readme; see #146

Ichoran commented 7 years ago

@MichaelCurrie - I'm not familiar with any transformations you might do in the Python code. This PR said that cx, cy should no longer be used as an offset for x, y values (i.e. only ox, oy would do that). Does this impact the code, or were these values always just passed on to the user? Either way, does the latest Python PR solve this issue?

MichaelCurrie commented 7 years ago

I believe it does impact the code: I believe the code will offset x,y by cx,cy if ox,oy are not present, but I will need to check this in more detail.

The latest Python PR did not solve this issue, if it is indeed present; this and contours are what's remaining for me to finish the Python 1.2 release. I'll be able to get to this next week.