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

Meaning of each section in Plots view. #1635

Closed
daavoo opened this issue Apr 29, 2022 · 23 comments
Closed

Meaning of each section in Plots view. #1635

daavoo opened this issue Apr 29, 2022 · 23 comments
Assignees
Labels
🎨 design Needs design input or is being actively worked on priority-p1 Regular product backlog question Further information is requested

Comments

@daavoo
Copy link
Contributor

daavoo commented Apr 29, 2022

I don't fully understand the purposes of each section. Maybe we could have a tooltip explaining?

Figma specs

From the demo project:

Plots

Captura de Pantalla 2022-04-29 a las 19 35 38

I see predictions.json and training_metrics for all revisions.

Comparison

Captura de Pantalla 2022-04-29 a las 19 35 53

I see misclassified.jpg so I assume this is for logged images?

The UI appears different, I can drag and drop revisions but this is not available in the Plots section (i.e. I would like to be able to reorder the confusion matrix like I reorder the image)

Experiment checkpoints

Captura de Pantalla 2022-04-29 a las 19 36 06

I see the same content as Plots but with different axis labels and without the workspace and main revisions.

@daavoo daavoo added question Further information is requested 🎨 design Needs design input or is being actively worked on labels Apr 29, 2022
@mattseddon
Copy link
Member

mattseddon commented May 1, 2022

@Davoo the purpose of each section is as follows:

Plots - all template-based plots from DVC (i.e everything that is not an image). I considered naming this section "templates". We can get all revisions for these plots because they are served up through plots diff.

Comparison - currently limited to images that can be compared. Again these come from plots diff.

Experiment Checkpoints - Custom line plots generated by the extension based on the information logged at each checkpoint. We get this information from exp show which means that we cannot easily get the required information for either the HEAD revision OR workspace (hence them being missing). These will look very similar to the plots generated by DVCLive as they are meant to be a drop-in replacement for the small numbers of users that use experiment checkpoints without DVCLive.

I can see why this can be confusing when we basically have "duplicate" plots across two sections but after the removal of the live section from dvc.yaml there is no longer a reliable way of determining whether or not a DVC project is using DVCLive. If there are duplicates then the user can simply close that section.

I can also see why this would be confusing if you are not familiar with the project.

It should be noted that you CAN drag and drop plots in the "Plots" and "Experiment Checkpoints" sections:

Screen.Recording.2022-05-02.at.9.23.51.am.mov

But (to the best of my knowledge) what you are asking for in terms of re-ordering the confusion matrix is not possible. The reason for this is that there is no way to re-order that template plot (I looked into this when trying to keep the order stable on live update). Also if we did this for that one template then I think we would be opening ourselves up to a lot of work trying to cater for every template out there in an ad-hoc manner. Studio considered making this possible by splitting the plot but then they would have ended up with the individual matrices being on different scales which makes them a lot less useful. For that reason they reverted to leaving the plot static and we've followed the approach.

Hope this all makes sense. Please LMK if anything is unclear.

@mattseddon
Copy link
Member

@maxagin, @yalozhkin it would be good to add an on hover or an info button per section to explain the purpose of the section. We do have the info codicon that we could use:

image

Can we please get a design for this interaction/behaviour. Thanks

@maxagin
Copy link
Contributor

maxagin commented May 2, 2022

@mattseddon Just an idea. Please let me know how you find it. Thanks

I think that if there is something that needs to be explained or clarified it is better to be visible. So here is the concept that reflects this.

Untitsssled-1

@maxagin maxagin self-assigned this May 2, 2022
@maxagin
Copy link
Contributor

maxagin commented May 3, 2022

Some subjects from today's discussion are added to the Notion doc. Thank you !
https://www.notion.so/iterative/MA-Plots-2687fd7b218742aaaf678b5db60bf1b7

@daavoo
Copy link
Contributor Author

daavoo commented May 3, 2022

after the removal of the live section from dvc.yaml there is no longer a reliable way of determining whether or not a DVC project is using DVCLive.

@mattseddon if I understand the implementation correctly, this would not only affect DVCLive users but any users logging scalars to both plots and metrics files at each step.

Regardless, do you think it would be valuable if DVCLive exposes an env var on instantiation in order to signal that it's being used?

@mattseddon
Copy link
Member

Regardless, do you think it would be valuable if DVCLive exposes an env var on instantiation in order to signal that it's being used?

I think it would be useful but I wouldn't prioritise it. It only solves a very specific edge case at the moment.

@shcheklein shcheklein added the priority-p1 Regular product backlog label May 4, 2022
@maxagin
Copy link
Contributor

maxagin commented May 4, 2022

@yalozhkin I remember that we discussed yesterday what would be the names of the sections for now. Please share the mockup if you have it or just the names. Thank you !

@maxagin
Copy link
Contributor

maxagin commented May 18, 2022

@mattseddon what is the status of this ticket? Is this idea can be useful #1635 (comment) ? Let me know PL

@mattseddon
Copy link
Member

mattseddon commented May 18, 2022

The status is that the updated section names and any helper text need to be added to the design shown in this figma. We will then be able to implement that design and close this ticket.

@shcheklein
Copy link
Member

@maxagin do we need / can we get this spec-ed out before the release?

@maxagin
Copy link
Contributor

maxagin commented Jun 4, 2022

can we get this spec-ed out before the release

Yes @shcheklein we can.

@maxagin
Copy link
Contributor

maxagin commented Jun 4, 2022

Hey, guys. I have published the specs. Please let me know if you have any comments or questions. Cheers

@shcheklein
Copy link
Member

@maxagin spec looks a bit different + comment is off there? (minor things since for this specific spec of course). Also, would it better to use an "i" icon instead - so that these explanation can be hidden for people who understand them already?

@mattseddon
Copy link
Member

@maxagin I need the actual titles and helper text not placeholder text.

@jorgeorpinel can you help out with the exact copy?

@shcheklein
Copy link
Member

@maxagin I need the actual titles and helper text not placeholder text.

that on me (or @jorgeorpinel can help with that), but let's first agree on the design please

@mattseddon
Copy link
Member

Also think the text should should not be shown all the time. Once I've read it once I will never want/need to see it again.

@maxagin
Copy link
Contributor

maxagin commented Jun 5, 2022

@shcheklein spec looks a bit different + comment is off there?

Sorry, can you please clarify what you mean by saying it looks different?

would it better to use an "i" icon instead

The original concept was more or less like on the attachment. However, what exactly to display will be related to the type of description we want to have. Some information is better to be shown all the time especially if we could have different descriptions.

Screen Shot 2022-06-04 at 10 29 37 PM

  1. I can add the icon and remove the description and
  2. I can add icon and keep the description

WDYT?

@shcheklein
Copy link
Member

I can add the icon and remove the description and

In this case it's the most appropriate. Since description is static and serves the purpose of explaining those sections.

Sorry, can you please clarify what you mean by saying it looks different?

Screen Shot 2022-06-04 at 8 27 04 PM

Screen Shot 2022-06-04 at 8 27 23 PM

Screen Shot 2022-06-04 at 8 28 56 PM

@maxagin
Copy link
Contributor

maxagin commented Jun 6, 2022

looks different

  1. Removed rounded corners, since in the VS Code we do not use this. I think even in terms of documentation we need to be consistent.
  2. The text colour changed from pink to dark. I do not see a reason to use pink for the documentation.
    Please let me know if you have any comments.

In this case it's the most appropriate. Since description is static

If I could see the real description it would help me to make a better decision or have the right arguments. Thanks

@mattseddon
Copy link
Member

This is what we currently have in the walkthrough for plots: https://github.com/iterative/vscode-dvc/blob/main/extension/resources/walkthrough/plots.md.

Note the section names have changed to Data Series, Images & Trends.

It is not final but will hopefully help to get you started.

@maxagin
Copy link
Contributor

maxagin commented Jun 6, 2022

Hey guys. I have updated the specs with all the changes. Please let me know WDYT. Thanks !

Figma spacs

@jorgeorpinel
Copy link
Contributor

Figma spacs

Sorry for late reply. I do not have access to this. Please share to jorge@iterative.ai . Thanks

@shcheklein
Copy link
Member

@jorgeorpinel see and review the text here please #1851

@maxagin maxagin removed their assignment Jun 24, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🎨 design Needs design input or is being actively worked on priority-p1 Regular product backlog question Further information is requested
Projects
None yet
Development

No branches or pull requests

6 participants