-
Notifications
You must be signed in to change notification settings - Fork 101
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
do not pre-load dataset version preview from string #642
Conversation
Deploying datachain-documentation with Cloudflare Pages
|
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #642 +/- ##
==========================================
+ Coverage 87.70% 87.73% +0.03%
==========================================
Files 112 112
Lines 10672 10676 +4
Branches 1437 1437
==========================================
+ Hits 9360 9367 +7
+ Misses 954 950 -4
- Partials 358 359 +1
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
bba502e
to
d1d7143
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.
🔥
src/datachain/dataset.py
Outdated
@cached_property | ||
def preview(self) -> Optional[list[dict]]: | ||
if isinstance(self._preview_data, str): | ||
return json.loads(self._preview_data) |
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.
Should we use orjson
as it is faster?
See #178
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 did consider but was scared... should be fine 😬
d1d7143
to
a94eac4
Compare
This PR ensures that we don't load a dataset version's preview to a dict from a string until it has been requested. This should mean that we can continue to use the pattern of loading all version for a dataset via
catalog.get_dataset
without taking the performance hit of loading all of the dataset version previews viajson.loads