Pull dataset from studio if not available locally #901
Conversation
If the following case are met, this will pull dataset from Studio. - User should be logged in to Studio. - The dataset or version doesn't exist in local - User has not pass studio=False to from_dataset. In such case, this will pull the dataset from studio before continuing further. The test is added to check for such behavior. Closes #874
Deploying datachain-documentation with
|
| Latest commit: |
c4cdeed
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://d641f349.datachain-documentation.pages.dev |
| Branch Preview URL: | https://amrit-from-dataset.datachain-documentation.pages.dev |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #901 +/- ##
=======================================
Coverage 87.69% 87.70%
=======================================
Files 130 130
Lines 11664 11683 +19
Branches 1586 1589 +3
=======================================
+ Hits 10229 10246 +17
+ Misses 1038 1037 -1
- Partials 397 400 +3
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
| @@ -1112,6 +1135,21 @@ def __iter__(self): | |||
| def __or__(self, other): | |||
| return self.union(other) | |||
|
|
|||
| def pull_dataset(self, name: str, version: Optional[int] = None) -> "DatasetRecord": | |||
| print("Dataset not found in local catalog, trying to get from studio") | |||
There was a problem hiding this comment.
let's use logger here in debug mode? @skshetry what is your take?
There was a problem hiding this comment.
I tried using logger with info here at beginning. But print seemed consistent with other similar messages. And also, we definitely want user to know we are trying to get it from Studio so that they can expect the delay in execution of the code.
| if not studio: | ||
| raise | ||
|
|
||
| if not is_token_set(): |
There was a problem hiding this comment.
Can we merge these 2 checks for studio and token in one line
| ds = self.catalog.get_dataset(name) | ||
| self.version = version or ds.latest_version | ||
| self.feature_schema = ds.get_version(self.version).feature_schema | ||
| try: |
There was a problem hiding this comment.
I think this logic should be in Catalog instead. Here we are putting a lot of custom code related to how and when we pull datasets into DatasetQuery which should not be it's domain IMO.
We currently have method Catalog.get_dataset_with_version_uuid(...) so I would create similar method with signature:
def get_dataset_with_version(name: str, version: int, studio=False) -> DatasetRecord
...This method would ensure to pull dataset from studio if the one with specific name + version doesn't exist locally and studio flag is set to True. Otherwise it would raise an Exception.
Then we wouldn't even need this special pull_dataset() method here, we would just call ds = self.catalog.get_dataset_with_version(...) and that's it.
| @@ -481,6 +481,7 @@ def from_dataset( | |||
| version: Optional[int] = None, | |||
| session: Optional[Session] = None, | |||
| settings: Optional[dict] = None, | |||
| studio: bool = True, | |||
There was a problem hiding this comment.
This flag is little bit ambiguous to me. If I'm user I wouldn't know if this means we fetch from studio first or locally? In any case, it's not clear that fetching from studio is just in case it doesn't exist locally.
If the following case are met, this will pull dataset from Studio.
In such case, this will pull the dataset from studio before continuing
further.
The test is added to check for such behavior.
Closes #874