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

docs/studio: top-level plots #4121

Merged
merged 3 commits into from
Dec 7, 2022
Merged

Conversation

ssachkovskaya
Copy link
Contributor

Related to https://github.com/iterative/studio/issues/4369

@tapadipti I decided not to add lots of text about the feature as it is already described in DVC in details. The idea is to mention that it is supported by Studio and give a link to the description.

@ssachkovskaya ssachkovskaya self-assigned this Nov 18, 2022
@shcheklein shcheklein temporarily deployed to dvc-org-feature-4369-st-pi7koe November 18, 2022 14:08 Inactive
@github-actions
Copy link
Contributor

github-actions bot commented Nov 18, 2022

679faa3

Link Check Report

There were no links to check!

@shcheklein shcheklein temporarily deployed to dvc-org-feature-4369-st-pi7koe November 21, 2022 04:33 Inactive
Copy link
Contributor

@tapadipti tapadipti left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @ssachkovskaya. I've included top-level plots in the example snippet and updated the text a bit. PTAL.

@tapadipti tapadipti added the C: studio Content of /doc/studio; github.com/iterative/studio/labels/A%3A%20help%20%26%20docs%20%26%20media label Nov 21, 2022
Copy link
Contributor

@jorgeorpinel jorgeorpinel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Proposed simplification 👇🏼

Comment on lines +69 to +71
directly,
- data from `runtime_logs/logs.json` will be rendered using the default (linear)
template.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
directly,
- data from `runtime_logs/logs.json` will be rendered using the default (linear)
template.
2. Images in `output/misclassified_samples` will be displayed directly (a single
image file could be used instead).

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jorgeorpinel I didn't make this change as it's not clear a single image file could be used instead "of what?".

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Instead of "Images in output/misclassified_samples". Could also rephrase that to "in the output/misclassified_samples directory", if that's clearer.

Comment on lines +73 to 75
For images, you can also specify a single image file (eg,
`output/misclassified_sample1.png`) instead of a directory.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
For images, you can also specify a single image file (eg,
`output/misclassified_sample1.png`) instead of a directory.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Didn't change this. Pls see above comment.

@jorgeorpinel
Copy link
Contributor

jorgeorpinel commented Nov 24, 2022

it is already described in DVC in details. The idea is to mention that it is supported by Studio

Even with the simplifications I proposed, this doc mainly documents DVC. We could probably remove everything after the numbered list of Types of plots and before How to generate plots.

and give a link to the description

I see a link to https://dvc.org/doc/command-reference/plots in the doc. Should probably go here instead: https://dvc.org/doc/user-guide/experiment-management/visualizing-plots

@jorgeorpinel
Copy link
Contributor

jorgeorpinel commented Nov 24, 2022

Q: what's special about flexible plots in Studio? Does it bring new UI features, screens, etc?

Also, are we going to call them "flexible" ? I think it makes more sense in Studio docs as "top-level" refers to how you define them in dvc.yaml (which is more for DVC docs to say).

@shcheklein shcheklein temporarily deployed to dvc-org-feature-4369-st-pi7koe November 29, 2022 05:44 Inactive
@tapadipti
Copy link
Contributor

Also, are we going to call them "flexible" ? I think it makes more sense in Studio docs

@jorgeorpinel could you pls elaborate what "flexible" means here. Then, we can decide on this. In one discussion I had with @pared, "top-level" was the decided term.

@jorgeorpinel
Copy link
Contributor

jorgeorpinel commented Nov 30, 2022

@tapadipti I think that the point of moving the plots definition to the top-level of dvc.yaml (which is an implementation detail) is that you can be more flexible with the data sources. See iterative/dvc#7086, iterative/example-repos-dev#117, et al.

BTW, I think the plan is to face out stage-level plot definitions so we probably won't even need the term "flexible" eventually, maybe we can completely avoid special terms in Studio docs, and we can just call them all plots (and link to DVC docs for the details).

That said, what's special about flexible plots in Studio? I'm still trying to understand why we're documenting this DVC feature in Studio docs.

@tapadipti
Copy link
Contributor

what's special about flexible plots in Studio? I'm still trying to understand why we're documenting this DVC feature in Studio docs.

Nothing special about it here, afaik. We're documenting it here so that people understand how they need to codify their plots in dvc if they want to visualize them in Studio. All DVC features that are directly relevant to Studio are documented to some extend in the Studio docs, and we redirect users to the dvc docs for more details.

@tapadipti
Copy link
Contributor

@jorgeorpinel I'm merging this PR, since the feature has been live in Studio for quite some time now. If there are more pending issues, we can address them in a separate PR.

@tapadipti tapadipti merged commit 679faa3 into main Dec 7, 2022
@tapadipti tapadipti deleted the feature/4369-studio-top-level-plots branch December 7, 2022 12:07
@jorgeorpinel
Copy link
Contributor

jorgeorpinel commented Dec 22, 2022

@tapadipti my main pending concern is that I think we may not need to document this DVC feature in Studio at all. We can just mention plots, where they can come from and maybe how flexible they can be, and then link to https://dvc.org/doc/user-guide/project-structure/dvcyaml-files#plots for formalities.

I also think the qualifier "top-level" has very little meaning in the context of Studio, let's just call them all plots regardless of how DVC codified them?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C: studio Content of /doc/studio; github.com/iterative/studio/labels/A%3A%20help%20%26%20docs%20%26%20media
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants