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

Table.from_numpy crashes on Inf values #2788

Closed
nikicc opened this issue Nov 21, 2017 · 7 comments
Closed

Table.from_numpy crashes on Inf values #2788

nikicc opened this issue Nov 21, 2017 · 7 comments
Assignees
Labels
snack This will take an hour or two

Comments

@nikicc
Copy link
Contributor

nikicc commented Nov 21, 2017

Orange version

3.8.0.dev0+301d13a

Expected behavior

One can create Table with from_numpy that contains infinity values.

Actual behavior

ValueError Array contains infinity. is raised in _check_arrays, not really sure why.

Steps to reproduce the behavior

Pass numpy array with inf values to Table.from_numpy

Additional info (worksheets, data, screenshots, ...)
@astaric
Copy link
Member

astaric commented Nov 22, 2017

Do we really want to allow inf values in X/Y? IMHO they should be converted to nan (=unknowns) before the table is constructed.

@janezd
Copy link
Contributor

janezd commented Nov 22, 2017

I agree with @astaric. A table with inf values is a corrupted table.

@nikicc
Copy link
Contributor Author

nikicc commented Nov 24, 2017

When calculating new features out of existing ones, np.inf is not really the same as np.nan.

But if we don't support them, I will convert them to np.nan.

@nikicc
Copy link
Contributor Author

nikicc commented Nov 24, 2017

An argument for this is that ScatterPlot seems to be handling inf values (#2709).

@ajdapretnar
Copy link
Contributor

This need to be resolved. I vote for converting inf to nan. @lanzagar @BlazZupan if you agree, let us propose a solution so someone can implement this.

@lanzagar
Copy link
Contributor

I don't see enough benefit in supporting inf values for the amount of problems I expect we would encounter in various widgets, at least for now.
I think converting to nan is the most practical solution.

@janezd janezd added the needs discussion Core developers need to discuss the issue label Jan 25, 2019
@janezd
Copy link
Contributor

janezd commented Feb 1, 2019

Decision (by @lanzagar, @janezd and @janezd): we change infs to nans.

@janezd janezd removed the needs discussion Core developers need to discuss the issue label Feb 1, 2019
@janezd janezd added easy snack This will take an hour or two labels Feb 16, 2019
@thocevar thocevar self-assigned this Feb 22, 2019
@janezd janezd closed this as completed Feb 25, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
snack This will take an hour or two
Projects
None yet
Development

No branches or pull requests

7 participants