-
Notifications
You must be signed in to change notification settings - Fork 42
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: Fix case where df.peek would fail to execute even with force=True #511
Conversation
bigframes/core/tree_properties.py
Outdated
@@ -25,3 +29,12 @@ def is_trivially_executable(node: nodes.BigFrameNode) -> bool: | |||
|
|||
def local_only(node: nodes.BigFrameNode) -> bool: | |||
return all(isinstance(node, nodes.ReadLocalNode) for node in node.roots) | |||
|
|||
|
|||
@functools.cache |
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.
Would this get unwieldy with local data? I worry about that Feather data getting stored forever.
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.
Yeah, not sure why I put that cache decorator. Would take a really long time for the cache to get too big though. Removed and will figure out the whole property caching thing later.
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'm worried about cached behavior and peelable getting out of sync again. Any chance we could refactor to avoid the possibility of the null value after we call cached?
Added
Now forcing the materialization after caching. |
#511) * fix: Fix case where df.peek would fail to execute even with force=True * remove cache from peekable property * if force=True always peek after caching even if peeking inefficient
Thank you for opening a Pull Request! Before submitting your PR, there are a few things you can do to make sure it goes smoothly:
Fixes #<issue_number_goes_here> 🦕