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

add log_linear template #136

Closed
wants to merge 1 commit into from
Closed

add log_linear template #136

wants to merge 1 commit into from

Conversation

dberenbaum
Copy link
Contributor

Adds a log_linear template for log scale data:

Linear plot:

visualization(8)

Log linear plot (this PR):

visualization(9)

@shcheklein
Copy link
Member

I think it should be a switch in the same template, having a separate template doesn't solve the issue / move the needle.

@dberenbaum
Copy link
Contributor Author

🤔 I thought about it, but not sure I agree. Smoothing is something I may want to keep adjusting. In my experience, I wouldn't go back and forth between linear and log scale -- one is likely right for my data and the other isn't, and I don't want to have to reset the scale each time across each interface. Do you have more info or thoughts on how it should be used?

It also feels like the problem there might be more about how hard it is to switch templates today. If it were possible to choose a different template for the plot from the UI, would it still make more sense as a switch?

@shcheklein
Copy link
Member

🤔 I thought about it, but not sure I agree. Smoothing is something I may want to keep adjusting. In my experience, I wouldn't go back and forth between linear and log scale -- one is likely right for my data and the other isn't, and I don't want to have to reset the scale each time across each interface.

yes, an most likely you are right. But you don't want to change a YAML file to find that out. It should be a button to my mind even if stays the same all the time after.

It also feels like the problem there might be more about how hard it is to switch templates today. If it were possible to choose a different template for the plot from the UI, would it still make more sense as a switch?

yep, probably, It would become a different implementation towards the same solution I guess. Might be more complicated though to implement in all products.

@dberenbaum
Copy link
Contributor Author

The same is true for all plots config -- it would be better to adjust it by looking at the plot and making selections visually than by updating a yaml file -- choosing templates, titles, labels, and even x/y fields. Related to iterative/vscode-dvc#2524. It feels too difficult in general to configure plots.

@dberenbaum
Copy link
Contributor Author

Tried to redo it as a UI toggle but so far can't seem to get it working in vega-lite for some reason.

@dberenbaum
Copy link
Contributor Author

Opened vega/vega-lite#8974

@@ -598,6 +598,123 @@ class LinearTemplate(SmoothLinearTemplate):
DEFAULT_NAME = "linear"


class LogLinearTemplate(Template):
DEFAULT_NAME = "log_linear"
DEFAULT_CONTENT = {
Copy link
Contributor

Choose a reason for hiding this comment

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

minor: I think there might be a way to inhering from LinearTemplate to not have to define the entire default content again

@daavoo
Copy link
Contributor

daavoo commented Aug 7, 2023

@dberenbaum @shcheklein what is the status here?
No strong opinion from my side, adding a new template is ok. I do agree that this just highlights the symptom that configuring plots is too hard today.
IMO, some props like this toggle should be modified at the UI level (VSCode, Studio)

@dberenbaum
Copy link
Contributor Author

I'm fine to merge or close. I wouldn't put more time into it since I think we should start trying to move to plotly and freeze vega dev unless it's critical.

@shcheklein
Copy link
Member

I'm fine to merge or close. I wouldn't put more time into it since I think we should start trying to move to plotly and freeze vega dev unless it's critical.

I agree on that. At least if there is no simple quick way of enabling that inside the existing templates. Closing this then. But I'm fine with either decision also.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants