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

feat: add options to mean, median and variance #471

Merged
merged 25 commits into from
Jun 14, 2024

Conversation

EscapedGibbon
Copy link
Collaborator

close: #470

@EscapedGibbon EscapedGibbon linked an issue Jun 7, 2024 that may be closed by this pull request
@EscapedGibbon EscapedGibbon force-pushed the 470-imagemean-accept-point-as-option branch from 6c4eb1f to 86d1988 Compare June 7, 2024 10:03
@EscapedGibbon EscapedGibbon marked this pull request as ready for review June 7, 2024 10:07
Copy link
Contributor

@stropitek stropitek left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Variance will return the result of a 0-division if the array of points is empty. Did you think about what should happen in that case?

src/compute/variance.ts Outdated Show resolved Hide resolved
src/compute/mean.ts Outdated Show resolved Hide resolved
src/compute/mean.ts Outdated Show resolved Hide resolved
@stropitek
Copy link
Contributor

Variance will return the result of a 0-division if the array of points is empty. Did you think about what should happen in that case?

Same question is relevant for the other operations.

@EscapedGibbon
Copy link
Collaborator Author

Variance will return the result of a 0-division if the array of points is empty. Did you think about what should happen in that case?

Indeed this should throw. Should we also check the validity of a coordinate? Right now the user can input any value there, regardless whether the point belongs to an image or not.

@stropitek
Copy link
Contributor

Indeed this should throw.

I agree

Should we also check the validity of a coordinate? Right now the user can input any value there, regardless whether the point belongs to an image or not.

I think it should throw as well. To be efficient, I suggest that you first convert the points to indices so that you don't do it twice, checking their value (compare with size) and then use getValue safely.

Please add test cases for each of those scenarios (no points, invalid points).

src/compute/median.ts Outdated Show resolved Hide resolved
@EscapedGibbon EscapedGibbon marked this pull request as draft June 14, 2024 06:42
@EscapedGibbon EscapedGibbon marked this pull request as ready for review June 14, 2024 09:57
@EscapedGibbon EscapedGibbon merged commit 37db7e3 into main Jun 14, 2024
8 checks passed
@EscapedGibbon EscapedGibbon deleted the 470-imagemean-accept-point-as-option branch June 14, 2024 11:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Image.mean accept Point[] as option
2 participants