DEP: Issue FutureWarning when malformed records detected. #10543
Conversation
|
👍 |
numpy/core/records.py
Outdated
| # 2018-02-07, 1.14.1 | ||
| warnings.warn( | ||
| "Bad records, may be list of lists instead of list of tuples, " | ||
| "Trying to fix. In the future this will raise an error", |
There was a problem hiding this comment.
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."
There was a problem hiding this comment.
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...
There was a problem hiding this comment.
@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.
There was a problem hiding this comment.
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.
271b54f to
33c423e
Compare
|
Moved the warning to after the attempt to interpret as list of lists. |
|
LGTM, will merge in a little bit if there are no more comments. |
|
Merging, thanks Chuck! |
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.