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

behaviour of rectangle and polygon of skimage.draw to float inputs #4283

Closed
ajkswamy opened this issue Nov 5, 2019 · 6 comments · Fixed by #6501
Closed

behaviour of rectangle and polygon of skimage.draw to float inputs #4283

ajkswamy opened this issue Nov 5, 2019 · 6 comments · Fixed by #6501
Labels

Comments

@ajkswamy
Copy link

ajkswamy commented Nov 5, 2019

Description

When skimage.draw.rectangle gets float inputs for start and end, the output is also float, which is unexpected as the docs say that the outputs should be array of ints. Documentation needs clarity as to whether float inputs are allowed and if so, what the expected behavior is.

skimage.draw.polygon returns array of ints when r or c inputs are floats. However the behavior is not clear from the docs.

Way to reproduce

from skimage.draw import rectangle, polygon
rr, cc = rectangle(start=(1.5, 1.5), end=(6.5, 6.5))
print(rr)
print(cc)
rr, cc = polygon(r=(1.5, 5.5, 3.5), c=(2, 2, 3))
print(rr)
print(cc)

Output:

[[1.5 2.5 3.5 4.5 5.5 6.5]
 [1.5 2.5 3.5 4.5 5.5 6.5]
 [1.5 2.5 3.5 4.5 5.5 6.5]
 [1.5 2.5 3.5 4.5 5.5 6.5]
 [1.5 2.5 3.5 4.5 5.5 6.5]
 [1.5 2.5 3.5 4.5 5.5 6.5]]
[[1.5 1.5 1.5 1.5 1.5 1.5]
 [2.5 2.5 2.5 2.5 2.5 2.5]
 [3.5 3.5 3.5 3.5 3.5 3.5]
 [4.5 4.5 4.5 4.5 4.5 4.5]
 [5.5 5.5 5.5 5.5 5.5 5.5]
 [6.5 6.5 6.5 6.5 6.5 6.5]]
[2 3 4 5]
[2 2 2 2]

Version information

# Paste the output of the following python commands
from __future__ import print_function
import sys; print(sys.version)
import platform; print(platform.platform())
import skimage; print("scikit-image version: {}".format(skimage.__version__))
import numpy; print("numpy version: {}".format(numpy.__version__))
3.7.4 (default, Oct  4 2019, 06:57:26) 
[GCC 9.2.0]
Linux-5.3.6-arch1-1-ARCH-x86_64-with-arch
scikit-image version: 0.16.2
numpy version: 1.17.1
@jni
Copy link
Member

jni commented Nov 5, 2019

@ajkswamy You're absolutely right! I'm unsure yet whether we should change the documentation or the function's behaviour, but something should be fixed for sure. Thanks for the report!

@richard-moss
Copy link

richard-moss commented Jan 31, 2020

Hi all,

I just noticed this exact problem (came here to report it too).

@jni I would say that this is a bug, and so should be fixed:

  • It's counter to what I think is the intent of the function, which is to return coordinates of pixels; ie something you can pass as indicies into a numpy array. This obviously doesn't work when it returns floats.
  • The above is seen not just in the documentation for the description, 'Returns' but also the Examples, where the "pass the coordinates into a numpy array" is the primary use case given in the example.
  • It's counter to what the behaviour is for the other draw functions (polygon, ellipse etc)

It looks like the somewhat easy work-around right now is to convert the output to int arrays (which will do an int floor, which will give you a 1 pixel error half the time, or do a rounding to nearest int conversion

@jni
Copy link
Member

jni commented Jan 31, 2020

@richard-moss thank you for your input! It's extremely valuable to hear the perspective of our users. Would you be comfortable submitting a fix in a pull request? I think using np.round(...).astype(int) is the right approach, before clipping to make sure all the coordinates are inside the image shape.

@richard-moss
Copy link

@jni I'll try to get to it the next few days.

That approach does work fine (I'm using it in my code as the workaround), but I'll also check how polygon() etc are doing it right now (that do return int's when passed floats) to make sure it's in line with their implementation.

@rfezzani rfezzani added the 📄 type: Documentation Updates, fixes and additions to documentation label Oct 18, 2021
@fissoreg
Copy link

Hey @richard-moss @jni !

Happy to see this issue, as I stumbled on the same problem. Any news about the fix? I would be happy to help, if needed :)

jstorrs added a commit to jstorrs/scikit-image that referenced this issue Sep 1, 2022
skimage.draw.rectangle(start,end) with float input produces float output
that cannot be used to index which is unexpected behavior compared to
the other skimage.draw.* functions. See discussion at issue scikit-image#4283

scikit-image#4283

This patch implements np.round(...).astype(int) as suggested by @jni.
Users who desire different behavior for float input can explicitly
convert start, end to int as they desire before calling
skimage.draw.rectangle(). np.round(...).astype(int) seems a reasonable
default that behaves similarly to the other draw functions.
@jstorrs
Copy link
Contributor

jstorrs commented Sep 1, 2022

I also just ran into this and didn't expect to not be able to index with the output. From what I can tell polygon() also applies np.round(...).astype(int) to vertices.

When I started working on the patch I began to question whether the desired behavior should be to produce a rectangle fully contained in the requested rectangle. Or a rectangle that fully contains the requested rectangle, etc. But I think that in the end for rectangle() it is easy for a user to do something different and send their own ints when needed rather than floats if desired. The np.round() approach at least guarantees that something reasonable happens by default rather than producing results that can't be used to index.

@rfezzani rfezzani added 🐛 Bug and removed 📄 type: Documentation Updates, fixes and additions to documentation labels Sep 1, 2022
rfezzani pushed a commit that referenced this issue Sep 2, 2022
* Support float input to skimage.draw.rectangle() [#4283]

skimage.draw.rectangle(start,end) with float input produces float output
that cannot be used to index which is unexpected behavior compared to
the other skimage.draw.* functions. See discussion at issue #4283

#4283

This patch implements np.round(...).astype(int) as suggested by @jni.
Users who desire different behavior for float input can explicitly
convert start, end to int as they desire before calling
skimage.draw.rectangle(). np.round(...).astype(int) seems a reasonable
default that behaves similarly to the other draw functions.

* Add test_rectangle_float_input
@lagru lagru added the 🐛 Bug label Sep 17, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants