Text box widget, take over of PR5375 #6988

Merged
merged 9 commits into from Sep 3, 2016

Conversation

Projects
None yet
8 participants
Member

fariza commented Aug 27, 2016

Friendly take over of #5375

mdboom added the needs_review label Aug 27, 2016

fariza changed the title from Text box widget to Text box widget, take over of https://github.com/matplotlib/matplotlib/pull/5375 Aug 27, 2016

fariza changed the title from Text box widget, take over of https://github.com/matplotlib/matplotlib/pull/5375 to Text box widget, take over of PR5375 Aug 27, 2016

Member

fariza commented Aug 27, 2016

@tacaswell here is the new PR, I just discover a small bug

Traceback (most recent call last):
  File "/home/fede/workspace/matplotlib/lib/matplotlib/backends/backend_gtk3.py", line 249, in key_press_event
    FigureCanvasBase.key_press_event(self, key, guiEvent=event)
  File "/home/fede/workspace/matplotlib/lib/matplotlib/backend_bases.py", line 1839, in key_press_event
    self.callbacks.process(s, event)
  File "/home/fede/workspace/matplotlib/lib/matplotlib/cbook.py", line 541, in process
    proxy(*args, **kwargs)
  File "/home/fede/workspace/matplotlib/lib/matplotlib/cbook.py", line 408, in __call__
    return mtd(*args, **kwargs)
  File "/home/fede/workspace/matplotlib/lib/matplotlib/backend_bases.py", line 2685, in key_press
    key_press_handler(event, self.canvas, self.canvas.toolbar)
  File "/home/fede/workspace/matplotlib/lib/matplotlib/backend_bases.py", line 2609, in key_press_handler
    if not (event.key in all):
TypeError: argument of type 'builtin_function_or_method' is not iterable

I will try to fix it next week

@Nodraak can you submit a PR against this one?

@smithsp can you create a PR from your branch https://github.com/smithsp/matplotlib/tree/text-box-widget to mine https://github.com/fariza/matplotlib/tree/text-box-widget

Contributor

Nodraak commented Aug 27, 2016

@fariza Here you go :)

I just discover a small bug

I'm not sure if all is supposed to be all_keys ... Looks like a silly typo

tacaswell added this to the 2.1 (next point release) milestone Aug 27, 2016

Contributor

QuadmasterXLII commented Aug 27, 2016

Hi! Thanks for the friendly takeover -- sorry for dropping off the face of the earth there

Member

fariza commented Aug 28, 2016

@Nodraak for the time being I didn't take the set_facecolor change because I wasn't able to reproduce the problem.

Member

fariza commented Aug 30, 2016

@tacaswell I don't think the failure is related.

Owner

jenshnielsen commented Aug 30, 2016

I merged #7006 which should fix the docs build error on Travis

jenshnielsen reopened this Aug 30, 2016

Member

fariza commented Aug 30, 2016

@jenshnielsen any idea on the appveyor fail?

Member

fariza commented Aug 30, 2016

@smithsp maybe I'm lost in the logs, but I don't see a relation between this PR changes and the failure

Contributor

smithsp commented Aug 30, 2016

@fariza
I agree that this PR is not the cause of appveyor failure. I guess that is why you pinged @jenshnielsen

Owner

jenshnielsen commented Aug 30, 2016

That is probably fixed by #7011

Member

fariza commented Aug 30, 2016

In that case, that's all from my side.
Any other question/suggestion/changes?

Ready to merge

Owner

jenshnielsen commented Aug 31, 2016

Power cycling again

jenshnielsen reopened this Aug 31, 2016

@tacaswell tacaswell merged commit 5f553bc into matplotlib:master Sep 3, 2016

3 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
coverage/coveralls Coverage increased (+0.1%) to 70.389%
Details

tacaswell removed the needs_review label Sep 3, 2016

Owner

tacaswell commented Sep 3, 2016

Thanks everyone for getting this across the finish line!

Contributor

QuadmasterXLII commented Sep 3, 2016

Thank you so much for your help!

On Sat, Sep 3, 2016 at 4:09 PM, Thomas A Caswell notifications@github.com
wrote:

Thanks everyone for getting this across the finish line!


You are receiving this because you commented.
Reply to this email directly, view it on GitHub
#6988 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/AGgnAXEV-VCZMSyXwpatin0GEH-X5vgPks5qmdQUgaJpZM4JusI0
.

This was referenced Sep 12, 2016

QuLogic added the widgets/UI label Sep 13, 2016

fariza deleted the fariza:text-box-widget branch Oct 22, 2016

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