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

Add a benchmark for img_as #4269

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

hmaarrfk
Copy link
Member

Description

I included naive implementations below. They probably aren't the best, but they would be what I would use if I were to roll my own.

xref: #4268

Checklist

For reviewers

  • Check that the PR title is short, concise, and will make sense 1 year
    later.
  • Check that new functions are imported in corresponding __init__.py.
  • Check that new features, API changes, and deprecations are mentioned in
    doc/release/release_dev.rst.
  • Consider backporting the PR with @meeseeksdev backport to v0.14.x

@pep8speaks
Copy link

pep8speaks commented Oct 27, 2019

Hello @hmaarrfk! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found:

Line 65:27: E221 multiple spaces before operator

Comment last updated at 2019-11-04 13:18:59 UTC

@sciunto
Copy link
Member

sciunto commented Oct 28, 2019

@hmaarrfk You included notebook, i guess it is unattended.

Copy link
Member

@sciunto sciunto left a comment

Choose a reason for hiding this comment

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

I'm just worried that one function will fail (without testing). Besides that, I'm +1.

benchmarks/benchmark_img_as.py Show resolved Hide resolved
return self.image.astype(dtype_out)
elif (np.issubdtype(dtype_in, np.floating)
and np.issubdtype(dtype_out, np.integer)):
imax = np.iinfo(dtype_out).max
Copy link
Member

Choose a reason for hiding this comment

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

I looked at this function np.iinfo and looks like it is working only with interger dtypes. In skimage.util.dtype, we also have the range specifications. Perhaps you can use that.

Copy link
Member Author

Choose a reason for hiding this comment

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

dtype_out is guaranteed to be an integer at this point.

I guess I'm trying to replicate the rint function. I've tested this internally, and it works quite well. I used linspace with about 1E6 points to see how the two compare for uint16 data types.

@hmaarrfk
Copy link
Member Author

hmaarrfk commented Nov 4, 2019

benchmark coverage was proposed in #3329

Probably needs a rebase and moving it to Azure?

Base automatically changed from master to main February 18, 2021 18:23
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.

None yet

3 participants