Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Add support for grids #23

Merged
merged 8 commits into from
Mar 2, 2019
Merged

Add support for grids #23

merged 8 commits into from
Mar 2, 2019

Conversation

simonrw
Copy link
Collaborator

@simonrw simonrw commented Feb 28, 2019

This support is basic so far, and only for continuous views for now.

The user can choose how many grid points they wish to use for each axis
(defaults to 3 each), and the colour of the grid lines (defaults to a
grey colour).

Additionally abstract two functions for drawing horizontal/vertical lines.

This support is basic so far, and only for continuous views for now.

The user can choose how many grid points they wish to use for each axis
(defaults to 3 each), and the colour of the grid lines (defaults to a
grey colour).
@milliams
Copy link
Owner

milliams commented Mar 1, 2019

Looks good 😄 We probably don't want vertical grid lines for discrete views anyway so it makes sense to focus on continuous first.

Could you add a line to the CHANGELOG for this?

@milliams milliams added this to the 0.4.0 milestone Mar 1, 2019
@simonrw
Copy link
Collaborator Author

simonrw commented Mar 1, 2019

Update changelog

@milliams
Copy link
Owner

milliams commented Mar 1, 2019

I'm happy to merge this if you're ready?

@simonrw
Copy link
Collaborator Author

simonrw commented Mar 1, 2019

I think I'd like to have it at least implemented for CategoricalViews on the y axis. Hang on...

simonrw added 2 commits March 2, 2019 09:06
Categorical views only show horizontal lines, whereas continuous views
show lines in both directions.
This puts the data in front of the grid, which is more akin to how
matplotlib renders grids. Grids should be relatively low in the z axis.
@simonrw
Copy link
Collaborator Author

simonrw commented Mar 2, 2019

There we go, horizontal grid lines for CategoricalViews

I've also dropped the grid's z ordering down so that plot points and lines show up above the grid. @milliams how do you feel about this? matplotlib renders the grid lines above the plots. I feel rendering the grid underneath looks nicer.

matplotlib

matplotlib

plotlib

plotlib

@milliams
Copy link
Owner

milliams commented Mar 2, 2019

I think I prefer your style with the grid lines in the background and the data points on top.

@simonrw
Copy link
Collaborator Author

simonrw commented Mar 2, 2019

Glad to hear it. Just updated the documentation and fetched the latest changes from master.

It's still probably not feature complete, but we can build smaller features on top of this. I think we should merge this as it is.

Some thoughts I have:

  • configuring grid colour
  • basing grid spacing on the plot range
  • configuring what z-index the grid has
  • overriding our defaults for horizontal vs both
  • probably more...

@milliams
Copy link
Owner

milliams commented Mar 2, 2019

You're right that there're a bunch more features that we need to add to this in the future. Feel free to open issues for some of those improvements or we can just refer back to this PR.

@milliams milliams merged commit 2c0e516 into milliams:master Mar 2, 2019
@simonrw simonrw deleted the add-grid branch March 2, 2019 19:42
@simonrw simonrw mentioned this pull request Mar 3, 2019
7 tasks
# for free to join this conversation on GitHub. Already have an account? # to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants