grid-strategy icon indicating copy to clipboard operation
grid-strategy copied to clipboard

Add an option to pass an existing figure to get_grid

Open PeterMinin opened this issue 6 years ago • 3 comments

In case you decide that passing a figure in is ok (#54), here is an implementation :) Thank you for this project!

PeterMinin avatar Feb 28 '19 16:02 PeterMinin

So I think that gridspec.GridSpec doesn't actually require a figure, so I'm thinking maybe it's better to get out of the business of using figures at all if we can help it.

I have to take a closer look at the code to see what that will mean for the mechanism we're using to produce the GridSpec, though, and for how convenient the module is to use.

pganssle avatar Feb 28 '19 17:02 pganssle

I think in the long term it might be best to re-factor all the stuff that requires a figure out into some separate mechanism, to make it easier to re-use this with other plotting systems, but since we're so tightly tied to matplotlib at the moment, we can go ahead with this. No need to make the perfect the enemy of the good.

@PeterMinin Any chance you can add a test for this? I think the best way is to mock out gridspec.GridSpec and test that it is passed the correct figure object.

Edit: Some examples of similar mocking are in tests/test_grids.py, which is where the test for this new behavior should go as well.

pganssle avatar Mar 16 '19 15:03 pganssle

Yes, hopefully I'll be able to do it in a couple of days.

PeterMinin avatar Mar 19 '19 17:03 PeterMinin