Skip to content
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

Remove support for executing last expression in the script #360

Closed
skshetry opened this issue Aug 27, 2024 · 4 comments · Fixed by #407
Closed

Remove support for executing last expression in the script #360

skshetry opened this issue Aug 27, 2024 · 4 comments · Fixed by #407
Assignees
Labels
enhancement New feature or request priority-p1

Comments

@skshetry
Copy link
Member

No description provided.

@shcheklein shcheklein added priority-p1 enhancement New feature or request labels Aug 31, 2024
@skshetry skshetry self-assigned this Sep 2, 2024
@skshetry
Copy link
Member Author

skshetry commented Sep 2, 2024

A couple of questions:

  1. Do we need to show previews for multiple datasets, given that there can be multiple save()s within the script? Or, is it okay to show the last one that wins?

  2. Since we don't have any custom wrapper, we won't be able to show previews for temp datasets or persist()-ed dataset in datachain query command. I guess that would be okay? Same in Studio.

Since we won't be wrapping the datasets, the only way for us to know what datasets were created in the script is by using job_id.

@skshetry
Copy link
Member Author

skshetry commented Sep 2, 2024

Looks like we will still need some kind of wrapper for "metrics" to work.

@shcheklein
Copy link
Member

Do we need to show previews for multiple datasets, given that there can be multiple save()s within the script? Or, is it okay to show the last one that wins?

Yes, I think that's fine to show the last one.

Since we don't have any custom wrapper, we won't be able to show previews for temp datasets or persist()-ed dataset in datachain query command. I guess that would be okay? Same in Studio.

could you point to the code that wraps it / add a bit more details?

skshetry added a commit to skshetry/datachain that referenced this issue Sep 4, 2024
Reverts iterative/dvcx#1443.

Since we want to make save explicits via users, and are dropping last expression
wrapping thing in iterative#360, we won't have any mechanism to support this feature.
skshetry added a commit to skshetry/datachain that referenced this issue Sep 4, 2024
Reverts iterative/dvcx#1443.

Since we want to make save explicits via users, and are dropping last expression
wrapping thing in iterative#360, we won't have any mechanism to support this feature.
skshetry added a commit to skshetry/datachain that referenced this issue Sep 4, 2024
Reverts iterative/dvcx#1443.

Since we want to make save explicits via users, and are dropping last expression
wrapping thing in iterative#360, we won't have any mechanism to support this feature.
@skshetry
Copy link
Member Author

skshetry commented Sep 5, 2024

After #395, datachain no longer requires scripts to have a last expression at all.

In that case, we'll show a preview for the last created dataset from the given script.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request priority-p1
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants