image-js / image-js-typescript

Temporary repository to work on the migration of image-js to TypeScript
https://image-js.github.io/image-js-typescript/
MIT License
5 stars 5 forks source link

feat!: refactor points and add absolute coords calculation #472

Closed EscapedGibbon closed 2 months ago

EscapedGibbon commented 2 months ago

close: https://github.com/image-js/image-js-typescript/issues/469

targos commented 2 months ago

I didn't expect the generator to be a private method. I thought we discussed:

EscapedGibbon commented 2 months ago

I didn't expect the generator to be a private method. I thought we discussed:

  • .points(kind) generator function
  • .absolutePoints getter with cache
  • .relativePoints getter with cache

We decided that it would be easier that the generator would treat both cases for absolute and relative coordinates. And since the code for both getters became similar, we decided to put it into one getter with a parameter that lets the user choose what kind of coordinates one wants. The coordinates are still cached separately though.

stropitek commented 2 months ago

We decided that it would be easier that the generator would treat both cases for absolute and relative coordinates.

That is not contradicting what @targos said. The point is that it should not be private.

And since the code for both getters became similar, we decided to put it into one getter with a parameter that lets the user choose what kind of coordinates one wants.

It's no longer a "getter" if it takes parameters. Let's go with the initial idea: relativePoints and absolutePoints getters. Otherwise we would have to find another name to differentiate the generator from the function returning the cached points.

stropitek commented 2 months ago

API SGTM but the documentation explanations related to the new fields don't seem clear enough to me. We should discuss it IRL.

EscapedGibbon commented 2 months ago

I wrote "Images/Masks" instead of "RoiMap" for absolute coordinates because RoiMap doesn't have origin property.