/matplotlib

MAINT: TkAgg default backend depends on tkinter #7530

Merged
merged 3 commits into from Dec 1, 2016
+27 −7

Conversation

Reviewers

@tacaswell tacaswell

@QuLogic QuLogic

@NelleV NelleV

Assignees
No one assigned
Labels
GUI/tk
Projects

Done in Reviewing pull requests.

Milestone
  v2.0.0
4 participants
@matthew-brett @NelleV @QuLogic @tacaswell
@matthew-brett
Contributor

matthew-brett commented Nov 29, 2016

We now always build the TkAgg backend, because it does not depend on the
Tk libraries being on the building system.

This had the unwanted effect of making the TkAgg backend be the default
even if Python tkinter libraries were not installed.

Make a runtime_check function for the optional packages, that gives
True if the package can be used at run time, and use that to reject
TkAgg as default backend when tkinter not available.
@matthew-brett matthew-brett MAINT: TkAgg default backend depends on tkinter 
We now always build the TkAgg backend, because it does not depend on the
Tk libraries being on the building system.

This had the unwanted effect of making the TkAgg backend be the default
even if Python tkinter libraries were not installed.

Make a `runtime_check` function for the optional packages, that gives
True if the package can be used at run time, and use that to reject
TkAgg as default backend when tkinter not available.

Some checks were not successful

1 errored and 2 successful checks
continuous-integration/appveyor/pr — AppVeyor build cancelled
Details
continuous-integration/travis-ci/pr — The Travis CI build passed
Details
coverage/coveralls — Coverage remained the same at 61.886%
Details
603f06f

matthew-brett referenced this pull request Nov 29, 2016

Closed

Cannot Disable TkAgg Backend #7526

setupext.py
+ """
+ pkg_name = 'tkinter' if PY3min else 'Tkinter'
+ try:
+ __import__(pkg_name)
@tacaswell

tacaswell Nov 29, 2016

Owner

how far back can we use importlib?

@matthew-brett

matthew-brett Nov 29, 2016

Contributor

__import__ available in 2.6, I just checked.

@QuLogic

QuLogic Nov 29, 2016

Member

importlib is 2.7 and 3.1+.

@matthew-brett

matthew-brett Nov 29, 2016

Contributor

I think you're right - but maybe worth noting that I'm not using importlib, only the __import__ builtin.

@tacaswell

tacaswell Nov 29, 2016

Owner

If importlib is in 2.7 can we use that instead of __import__/

setup.py
- if default_backend is None:
- default_backend = package.name
+ if (isinstance(package, setupext.OptionalBackendPackage)
+ and package.runtime_check()
@tacaswell

tacaswell Nov 29, 2016

Owner

🚲 🏠 can you add an extra 4 spaces to these two lines so the conditional and the code can be separated?

@matthew-brett

matthew-brett Nov 29, 2016

Contributor

:) - done.

tacaswell changed the title from MAINT: TkAgg default backend depends on tkinter to [MRG+1] MAINT: TkAgg default backend depends on tkinter Nov 29, 2016

tacaswell added this to the 2.0.1 (next bug fix release) milestone Nov 29, 2016

@matthew-brett matthew-brett STY: add indentation to if conditionals 
Indentation to distinguish clauses in the if conditional from the if
block.

Some checks were not successful

1 errored and 2 successful checks
continuous-integration/appveyor/pr — AppVeyor build cancelled
Details
continuous-integration/travis-ci/pr — The Travis CI build passed
Details
coverage/coveralls — Coverage remained the same at 61.886%
Details
310f033

NelleV changed the title from [MRG+1] MAINT: TkAgg default backend depends on tkinter to [MRG+2] MAINT: TkAgg default backend depends on tkinter Nov 29, 2016

@NelleV

NelleV approved these changes Nov 29, 2016
View changes

Thanks @matthew-brett !

QuLogic added the GUI/tk label Nov 29, 2016

@NelleV
Contributor

NelleV commented Nov 29, 2016

I don't think the indentation change you've made are pep8 compliant (I am not entirely sure on that, so let's wait for the tests).
@QuLogic
Member

QuLogic commented Nov 29, 2016

The indentation looks correct to me, but the binary operator should be before the break.
@matthew-brett matthew-brett RF: use importlib for importing + style change 
Use importlib to test module import.

Put 'and' at end of line instead of beginning.
This commit was signed with a verified signature.
@matthew-brett matthew-brett Matthew Brett
GPG key ID: 0A6232F4F6401040 Learn about signing commits

Some checks were not successful

1 failing and 2 successful checks
continuous-integration/appveyor/pr — AppVeyor build failed
Details
continuous-integration/travis-ci/pr — The Travis CI build passed
Details
coverage/coveralls — Coverage remained the same at 61.886%
Details
677801a

@tacaswell tacaswell merged commit 45e4a46 into matplotlib:master Dec 1, 2016
2 of 3 checks passed

2 of 3 checks passed

continuous-integration/appveyor/pr AppVeyor build failed
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
coverage/coveralls Coverage remained the same at 61.886%
Details

@tacaswell tacaswell added a commit that referenced this pull request Dec 1, 2016

@tacaswell tacaswell Merge pull request #7530 from matthew-brett/tkagg-depends-on-tkinter 
BLD: TkAgg default backend depends on tkinter
 7322046
@tacaswell
Owner

tacaswell commented Dec 1, 2016

backported to v2.x as 7322046

Thanks for taking on all of this Tk work!

QuLogic changed the title from [MRG+2] MAINT: TkAgg default backend depends on tkinter to MAINT: TkAgg default backend depends on tkinter Dec 1, 2016

tacaswell referenced this pull request Dec 1, 2016

Merged

Don't work out packages to install if user requests information from setup.p #7538

@QuLogic QuLogic modified the milestone: 2.0.1 (next bug fix release), 2.0 (style change major release) Dec 7, 2016

matthew-brett referenced this pull request Jan 20, 2017

Closed

wheels not picking correct default backend #7874

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