-
-
Notifications
You must be signed in to change notification settings - Fork 18.4k
WARN: Only warn about inconsistent parsing if there are multiple non-null elements #52195
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
WARN: Only warn about inconsistent parsing if there are multiple non-null elements #52195
Conversation
686a72e
to
44d0e9f
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One nit otherwise lgtm
) | ||
# If there are multiple non-null elements, warn about | ||
# how parsing might not be consistent | ||
if tslib.first_non_null(arr[first_non_null + 1 :]) != -1: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Only minor nit I'd have is to add an explicit if first_non_null == len(arr)
check preceding this. It doesn't have any impact as long as this stays a purely Python function, but if someone tried to convert down to C I'm not sure how slicing beyond the length of the array would work
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ignore this. Thought this was a pyx file
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Lgtm
Thanks @MarcoGorelli |
…ing if there are multiple non-null elements
…t parsing if there are multiple non-null elements) (#52242) Backport PR #52195: WARN: Only warn about inconsistent parsing if there are multiple non-null elements Co-authored-by: Marco Edward Gorelli <[email protected]>
doc/source/whatsnew/vX.X.X.rst
file if fixing a bug or adding a new feature.Not sure this needs a whatsnew note as it could just be considered part of the PDEP4 changes, which haven't yet reached user-facing status
The idea here is to only warn if there are at least 2 non-null elements - otherwise, a single element can't be inconsistently-parsed :)