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

Make BoundingBox inputs integer #99

Merged
merged 2 commits into from
Jan 10, 2017

Conversation

larrybradley
Copy link
Member

This is a followup to #97 to ensure the inputs to BoundingBox are integer.

Note that int() is called on the inputs, which means floats will be silently truncated. Do we also want to raise an exception for floats where e.g., int(ixmin) != ixmin?

@larrybradley larrybradley changed the title Bbox input Make BoundingBox input integer Jan 9, 2017
@larrybradley larrybradley changed the title Make BoundingBox input integer Make BoundingBox inputs integer Jan 9, 2017
@keflavich
Copy link
Contributor

For consistency with numpy's new behavior, yes, we should raise an exception there. I preferred the old approach of just warning, which I used all the time, but it will probably benefit many people... including me... to be forced to select how floats are truncated.

@cdeil
Copy link
Member

cdeil commented Jan 9, 2017

+1 to be strict on inputs and raise errors. Numpy now raises warnings where floats are used as indices (because I guess they can't throw errors to keep backward compat).

@keflavich
Copy link
Contributor

@cdeil I'm using numpy 1.12-dev, and I get exceptions if I try to use floats:

In [6]: np.__version__
Out[6]: '1.12.0.dev0+47c18a0'

In [7]: x = np.arange(5)

In [8]: x[2.0:3.0]
Traceback (most recent call last):
  File "<ipython-input-8-82b00ecaa918>", line 1, in <module>
    x[2.0:3.0]
TypeError: slice indices must be integers or None or have an __index__ method

@larrybradley
Copy link
Member Author

OK, done. Input floats (even e.g. 3.0) will now raise a TypeError.

@cdeil
Copy link
Member

cdeil commented Jan 9, 2017

I'm still on Numpy 1.11 ... there it's a VisibleDeprecationWarning:

In [1]: import numpy as np

In [2]: np.__version__
Out[2]: '1.11.3'

In [3]: x = np.arange(5)

In [4]: x[2.0:3.0]
/opt/local/bin/ipython:1: VisibleDeprecationWarning: using a non-integer number instead of an integer will result in an error in the future
  #!/opt/local/Library/Frameworks/Python.framework/Versions/3.5/bin/python3.5
Out[4]: array([2])

@larrybradley larrybradley merged commit 85b1a28 into astropy:master Jan 10, 2017
@larrybradley larrybradley deleted the bbox_input branch January 10, 2017 01:47
@cdeil cdeil added this to the 0.2 milestone Jun 17, 2019
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