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

Update custom plot spec #3634

Merged
merged 3 commits into from
Apr 5, 2023
Merged

Update custom plot spec #3634

merged 3 commits into from
Apr 5, 2023

Conversation

julieg18
Copy link
Contributor

@julieg18 julieg18 commented Apr 4, 2023

  • have metric vs param plot use scatter plot instead of linear plot

Demo

https://user-images.githubusercontent.com/43496356/229911951-9abe154b-3fd2-4145-a9e6-0baaa748e980.mov

Screen.Recording.2023-04-05.at.9.30.27.AM.mov

To Do

  • Fill marks and look into making marks for visible

Part of #3373

* have metric vs param plot use scatter plot
@julieg18 julieg18 added the product PR that affects product label Apr 4, 2023
@julieg18 julieg18 self-assigned this Apr 4, 2023
}
}
]
mark: {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I went with a scatter plot for now, but there was some discussion on what plot to use in #3373. Other options we could use are in demos there.

@shcheklein
Copy link
Member

@dberenbaum do you remember why we use non filled shape (circle) for the scatter plot template - it might be more visible / readable to fill them, wdyt?

@julieg18 julieg18 marked this pull request as ready for review April 4, 2023 21:00
}
}
})
} as CustomPlotsData
Copy link
Member

Choose a reason for hiding this comment

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

🎉

Copy link
Member

@mattseddon mattseddon left a comment

Choose a reason for hiding this comment

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

Let's fill in the marks/points and then ship this.

calculate: "format(datum['y'],'.5f')"
filled: true,
size: 60,
type: 'point'
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Turns out just filling point or using circle make the marks even smaller:

Screenshot 2023-04-05 at 9 09 11 AM

I ended up increasing the size so that the filled points would have a similar width to none filled points:

Screenshot 2023-04-05 at 9 13 40 AM

Screenshot 2023-04-05 at 9 12 08 AM

@julieg18 julieg18 enabled auto-merge (squash) April 5, 2023 14:37
@codeclimate
Copy link

codeclimate bot commented Apr 5, 2023

Code Climate has analyzed commit d150f2a and detected 0 issues on this pull request.

The test coverage on the diff in this pull request is 100.0% (85% is the threshold).

This pull request will bring the total coverage in the repository to 94.8% (0.0% change).

View more on Code Climate.

@julieg18 julieg18 merged commit 4a574ec into main Apr 5, 2023
@julieg18 julieg18 deleted the update-custom-plot-spec branch April 5, 2023 14:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
product PR that affects product
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants