Skip to content

Conversation

@fariza
Copy link

@fariza fariza commented May 29, 2015

This is the implementation of the Configure Subplot Tool, to use the "Generic" Window capabilities

@fariza
Copy link
Author

fariza commented Jun 4, 2015

I don't understand why more commits appear here, when first created the PR, there was only my commit.

@OceanWolf did you have time to check this?

@OceanWolf
Copy link
Owner

Hmm, no I only just saw it that you had made this 2nd PR. I did a rebase last night of the main branch. Do you want to try rebasing from that?

@OceanWolf
Copy link
Owner

I think we should add this separately later as we also need to remove this from TkAgg, for now lets use it just to make sure we don't miss anything here.

Why a ToggleTool? Before we had the functionality of the dialog raising to the top whenever the tool got activated, I can see loosing that functionality might annoy people.

@fariza
Copy link
Author

fariza commented Jun 8, 2015

I wanted to include it here just so we don't have to port the Subplot Tool for all the backends that don't have MEP22 stuff.

@OceanWolf
Copy link
Owner

I understand that, but I would rather leave that for just after we get the main PR landed:

  1. Get MEP27 landed for GTK3.
  2. Finish MEP27 for TkAgg (virtually finished) and port Subplot Tool for both TkAgg and GTK3 in the TkAgg MEP27 PR.
  3. Get MEP22 and MEP 27 working for other backends (note that we can work on the PRs for the other backends even without the TkAgg PR landed).

I think this way we:
a) don't break TkAgg for MEP27 in this PR (even though we will fix it in the next PR) and
b) we have fewer changes to get reviewed, making the review process faster.

@fariza
Copy link
Author

fariza commented Jun 25, 2015

Now that it's decided that MEP27 and MEP22 will work together exclusively do we incorporate this from the begining?

@OceanWolf
Copy link
Owner

Yes, but as I say above I want to put it in TkAgg's MEP27 conversion PR, hopefully it will only take a few hours after GTK3 has landed to landed to get TkAgg to land as only a hundred odd lines of code and adds nothing controversial on top of the initial GTK3 PR.

Besides, we can work on the other PRs and just ignore this tool until the TkAgg PR lands? We just now need the examples in place and then we can do a last call for comments :). Can you do the examples as you know what you want to see? Meanwhile I work on a similar PR to this one branched directly from master for the SaveFigureTool.

@OceanWolf
Copy link
Owner

Silly me, I think we could have merged it anyway, but now even better with the improved MEP22 code in MEP27.

One thing, I want to leave in ConfigureSubplotsBase for now, and remove it in the TkAgg PR, at present, TkAgg will just overwrite the tool. Do you want to remake this PR so I can merge it, or should I just make the adjustments myself? I will compare the differences between what you do here and my previous attempt at this back in the MEP22 PR (that I abandoned in order to begin work on MEP27 so we could make this tool properly).

@fariza
Copy link
Author

fariza commented Jun 25, 2015

If you have the time, go ahead.

@fariza
Copy link
Author

fariza commented Jun 30, 2015

This one is done right? so I can remove this PR

@OceanWolf
Copy link
Owner

Yup, all done, I went for a cross between this one and my original PR from MEP22.

OceanWolf pushed a commit that referenced this pull request Jul 14, 2015
FIX: respect alpha on alt face color
@OceanWolf OceanWolf force-pushed the backend-refactor branch 3 times, most recently from 620efb9 to d842ee8 Compare September 22, 2015 23:07
@fariza fariza closed this Dec 4, 2015
OceanWolf pushed a commit that referenced this pull request Sep 30, 2016
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.

2 participants