Skip to content

DEP: Issue FutureWarning when malformed records detected. - #10543

Merged
ahaldane merged 1 commit into
numpy:masterfrom
charris:warn-malformed-data
Feb 9, 2018
Merged

ahaldane merged 1 commit into
numpy:masterfrom
charris:warn-malformed-data

Conversation

@charris

@charris charris commented Feb 8, 2018

Copy link
Copy Markdown
Member

Currently records specified as lists of lists instead of lists of tuples
are caught and fixed up in fromrecords. Unfortunately, the detection is
failing due to various fixes in the records module. The failure is
worked around by generalizing the detection. A FutureWarning that such
record specifications will result in an error in the future is also
emitted.

Fixes #10344.

Note that is no longer needed for 1.14.1, but a backport is suggested just to get the warning out there.

@charris charris added this to the 1.15.0 release milestone Feb 8, 2018
@charris charris added the 09 - Backport-Candidate PRs tagged should be backported label Feb 8, 2018
@charris charris modified the milestones: 1.15.0 release, 1.14.1 release Feb 8, 2018
@oleksandr-pavlyk

Copy link
Copy Markdown
Contributor

👍

Comment thread numpy/core/records.py Outdated

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.

I understand but I think it might be a bit unclear. What about "fromrecords expected a list of tuples but got a list of lists. In the future this will raise an error, please update your code to use a list of tuples."

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.

Should this warning be emitted if the below code fails?

@ahaldane: may be a list of lists. Who knows why the TypeError was actually thrown...

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

@eric-wieser I thought getting both a warning followed by an error wouldn't be too bad if a bit ugly. The alternative is nested try blocks, which I suppose isn't so bad either.

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.

ACtually, warning followed by error is better - if the user has warnings as errors, then they'll see the more useful warning, rather than a more cryptic error.

Currently records specified as lists of lists instead of lists of tuples
are caught and fixed up in fromrecords. Unfortunately, the detection is
failing due to various fixes in the records module. The failure is
worked around by generalizing the detection. A FutureWarning that such
record specifications will result in an error in the future is also
emitted.

Fixes numpy#10344.
@charris
charris force-pushed the warn-malformed-data branch from 271b54f to 33c423e Compare February 8, 2018 19:54
@charris

charris commented Feb 8, 2018

Copy link
Copy Markdown
Member Author

Moved the warning to after the attempt to interpret as list of lists.

@ahaldane

ahaldane commented Feb 8, 2018

Copy link
Copy Markdown
Member

LGTM, will merge in a little bit if there are no more comments.

@ahaldane

ahaldane commented Feb 9, 2018

Copy link
Copy Markdown
Member

Merging, thanks Chuck!

@ahaldane
ahaldane merged commit b429be3 into numpy:master Feb 9, 2018
@charris charris removed the 09 - Backport-Candidate PRs tagged should be backported label Feb 9, 2018
@charris charris removed this from the 1.14.1 release milestone Feb 9, 2018
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.

New in NumPy 1.14: failures in PyTables

4 participants