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

Rough implementation of GridSpec #31

Open
wants to merge 2 commits into
base: master
from

Conversation

Projects
None yet
3 participants
@philippjfr
Copy link
Contributor

philippjfr commented Sep 6, 2018

Implements a GridSpec layout which allows declaring an NxM grid and assigning objects to the grid cells. Here is an example of a 3x3 grid:

screen shot 2018-09-06 at 6 33 17 pm

  • Invert indexing coordinates (rows then columns)
  • Improve width/height setting logic
  • Fix vertical margin issues
  • Add unit tests
  • Add documentation
Philipp Rudiger Philipp Rudiger
@jbednar

This comment has been minimized.

Copy link
Contributor

jbednar commented Sep 6, 2018

Wow, you are on a roll!

@jbednar

This comment has been minimized.

Copy link
Contributor

jbednar commented Sep 6, 2018

What happens when a grid cell is empty?

@jbednar

This comment has been minimized.

Copy link
Contributor

jbednar commented Sep 6, 2018

Would it be possible for the resulting grid to span the entire page area available (in a server context) or the entire width available (in a notebook context), and to responsively resize, with each item in a grid cell centered by default? That would be an immediate decent-looking and usable dashboard with very little work for users.

@philippjfr

This comment has been minimized.

Copy link
Contributor Author

philippjfr commented Sep 6, 2018

What happens when a grid cell is empty?

screen shot 2018-09-06 at 10 35 08 pm

Would it be possible for the resulting grid to span the entire page area available (in a server context) or the entire width available (in a notebook context), and to responsively resize, with each item in a grid cell centered by default?

I don't think this will work right now, but the new layout work in bokeh at least theoretically makes that possible if all the other issues can be resolved.

@jbednar

This comment has been minimized.

Copy link
Contributor

jbednar commented Sep 6, 2018

Sounds good, thanks.

@mattpap

This comment has been minimized.

Copy link

mattpap commented Sep 17, 2018

btw. bokeh implements its own variation of GridSpec, though it's just an API helper rather than a type of layout.

@philippjfr

This comment has been minimized.

Copy link
Contributor Author

philippjfr commented Sep 17, 2018

Thanks, I tried the bokeh GridSpec initially but it didn't do quite what we needed. That said I can't recall why, so I should go back before this is merged and compare/contrast the two implementations.

@mattpap

This comment has been minimized.

Copy link

mattpap commented Sep 17, 2018

GridSpec was added for a very particular task in one of bokeh's examples (removed at this point), so refining it is an option, especially there are no tests for it and it wasn't every publicized properly.

@philippjfr

This comment has been minimized.

Copy link
Contributor Author

philippjfr commented Sep 17, 2018

Okay, I remember now. One of the main usecases for a GridSpec is to assign individual plots to multiple grid cells, the implementation currently in bokeh does not seem to allow for that. When assigning to multiple cells at once you have to provide a list of plots, so really it's just a different way of building the grid yourself and works nothing like matplotlib's GridSpec.

@jbednar

This comment has been minimized.

Copy link
Contributor

jbednar commented Sep 17, 2018

Sounds like once panel's GridSpec is solid, Bokeh's GridSpec should either be removed with a pointer to use Panel instead, or else panel's GridSpec should be split into a Bokeh-only implementation with a panel-specific extension on top (i.e. one that understands Panel objects, not just Bokeh models).

@philippjfr

This comment has been minimized.

Copy link
Contributor Author

philippjfr commented Sep 17, 2018

We should definitely consider that. One reason why it might be weird to do that is that this implementation of GridSpec combines both layout and sizing, i.e. inserting an object into the grid defines the size of the plot. This is what makes it possible and what it means to assign a plot/model to multiple grid cells. It may be weird to mess with the sizes of bokeh models directly like that if this were added to bokeh itself, but I'd be happy to file an issue on bokeh to start that discussion.

@philippjfr philippjfr force-pushed the master branch from 2cd74d0 to 871e24e Oct 12, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment