-
Notifications
You must be signed in to change notification settings - Fork 872
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
Fix ColumnAccessor caching of nrows if empty previously #15710
Fix ColumnAccessor caching of nrows if empty previously #15710
Conversation
new_ncols = len(self._data) | ||
self._clear_cache(old_ncols, new_ncols) |
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.
I think the invariant is new_ncols == old_ncols + 1
? So no need to compute len
here.
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.
Good call. Will swap to this
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.
hasattr
evaluates the property which is not what you wanted, I think.
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.
/merge
/merge |
Description
#14758 may have propagated a caching invalidation bug of the number of rows in a
ColumnAccessor
Previously the number of rows was cached and cleared only if an operation caused the
ColumnAccessor
to have no more columns.However, if the
ColumnAccessor
was empty and operation added new columns, the cached number of rows should have also been cleared.Checklist