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

Enhanced peak_search documentation #3582

Merged
merged 1 commit into from
Jan 6, 2022
Merged

Enhanced peak_search documentation #3582

merged 1 commit into from
Jan 6, 2022

Conversation

t20100
Copy link
Member

@t20100 t20100 commented Jan 5, 2022

This PR clarifiy the returned type of peak indices (float, not int as one can expect for indices).

closes #3579

This PR clarifiy the returned type of peak indices (`float`, not `int` as one can expect for indices).

closes #3579
@t20100 t20100 added this to the Next release milestone Jan 5, 2022
@t20100 t20100 requested a review from loichuder January 5, 2022 16:56
Copy link
Member

@loichuder loichuder left a comment

Choose a reason for hiding this comment

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

Thanks !

@t20100 t20100 merged commit 61f9c42 into master Jan 6, 2022
@t20100 t20100 deleted the t20100-patch-2 branch January 6, 2022 08:24
@vasole
Copy link
Member

vasole commented Jan 6, 2022

No time to provide feedback...

I would have said it returns the closest index as a float. To get an actual index the use of int(returned_value + 0.5) is recommended.

@t20100
Copy link
Member Author

t20100 commented Jan 6, 2022

From the look at the code I had, even if it is floats, values are always integers so there is no need to take care of rounding.
If I missed something, a PR is welcomed!

@vasole
Copy link
Member

vasole commented Jan 6, 2022

It is for the case we decide to return the weighted average of three channels. Now it is correct.

@t20100
Copy link
Member Author

t20100 commented Jan 6, 2022

OK, then we can update the documentation when this is changed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[silx.math.fit] peak_search returns peak indices as float
3 participants