Skip to content

ENH: Better error message when bins has float value in histogramdd. - #16129

Merged
mattip merged 9 commits into
numpy:masterfrom
MatteoRaso:master
May 7, 2020
Merged

mattip merged 9 commits into
numpy:masterfrom
MatteoRaso:master

Conversation

@MatteoRaso

@MatteoRaso MatteoRaso commented May 1, 2020 •

Copy link
Copy Markdown
Contributor

Fixes gh-15984

@MatteoRaso MatteoRaso changed the title Addressed Issue #15984 ENH: Addressed Issue #15984 May 1, 2020
@mattip mattip changed the title ENH: Addressed Issue #15984 ENH: better error message for float in bins in np.histogramdd May 1, 2020
@mattip

mattip commented May 1, 2020

Copy link
Copy Markdown
Member

CI failures not related.

The check is not very specific. Maybe there is a better way to check specifically for non-integer values in bins?

@eric-wieser

Copy link
Copy Markdown
Member

We could call operator.index directly here, rather than relying on linspace to call it for us.

@charris charris changed the title ENH: better error message for float in bins in np.histogramdd ENH: Better error message when bins has float value in np.histogramdd May 1, 2020
@charris charris changed the title ENH: Better error message when bins has float value in np.histogramdd ENH: Better error message when bins has float value in histogramdd. May 1, 2020
@charris charris closed this May 1, 2020
@charris charris reopened this May 1, 2020
@MatteoRaso

Copy link
Copy Markdown
Contributor Author

I changed it to explicitly check if bins[i] is an integer.

Comment thread numpy/lib/histograms.py Outdated
@mattip

mattip commented May 2, 2020

Copy link
Copy Markdown
Member

This is worse, because it now rejects np.int8 and similar

CI passed on this commit, even though it rejected valid values. Does that mean tests are missing?

@MatteoRaso
MatteoRaso requested a review from eric-wieser May 2, 2020 22:09
@MatteoRaso

Copy link
Copy Markdown
Contributor Author

The azure-pipeline tests are saying that it can't install MinGW, which is why it's failing. Is there any way to rerun the test on my code?

@seberg seberg closed this May 2, 2020
@seberg seberg reopened this May 2, 2020
@MatteoRaso MatteoRaso closed this May 3, 2020
@seberg seberg reopened this May 3, 2020
@seberg

seberg commented May 3, 2020 •

Copy link
Copy Markdown
Member

Was the close intentional? We had some problem with the CI so it was necessary. Don't worry too much about it if one of the build fails for now.

EDIT: sorry, to be clear, the mingw error of course is unrelated... So if you get suck because of this, close/reopen may help (although we can likely just rerun the test in any case)

@MatteoRaso

Copy link
Copy Markdown
Contributor Author

The close was a mistake. Sorry about that.

Comment thread numpy/lib/histograms.py Outdated
Comment thread numpy/lib/histograms.py Outdated
@MatteoRaso MatteoRaso closed this May 5, 2020
@MatteoRaso MatteoRaso reopened this May 5, 2020
@MatteoRaso

Copy link
Copy Markdown
Contributor Author

Ok, the checks are working now.

@mattip mattip left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM.

Comment thread numpy/lib/histograms.py Outdated
Comment thread numpy/lib/histograms.py Outdated
Comment thread numpy/lib/histograms.py Outdated
@mattip
mattip merged commit 2f3b826 into numpy:master May 7, 2020
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.

np.histogramdd produces confusing errors when bins is a float

5 participants