Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork7.9k
Remove logic for optionally building Agg and TkAgg.#13075
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
2b3ba13 tod6aaaeeCompareWould it be better to actually make tkagg optional? |
It's been non-optional for years and no one complained; what would be the use case? Note that the build doesn't depend on any external headers or libraries. |
(In fact I'm tempted toalso make the OSX backend non-optional if someone can verify whether the cocoa headers are always installed with xcode...) |
punting to 3.2 as it has conflicts, in non-critical, and the fiddly to make sure we got it right. |
Rebased. Once again, note that always forcing the build of tkagg (which doesnot depend on the tk headers) has been the actual behavior for years now. |
It seems that moving Travis means I can't restart the broken build, so you'll have to rebase. |
They are actually not optional (that's the `force = True` setting whichoverrides the OptionalBackendPackage logic), so just make themSetupPackages and remove the corresponding (outdated) comments insetup.cfg.template.
anntzer commentedNov 4, 2019 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
rebased, build passes |
I remember something about someone having problems building matplotlibagainst a python that didn't have Tk. Could that be the reason why the codeis this way for TkAgg? …On Mon, Nov 4, 2019 at 6:14 AM Antony Lee ***@***.***> wrote: rebased — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub <#13075?email_source=notifications&email_token=AACHF6AW6OQXK4TQPDXGX3LQR772LA5CNFSM4GMYHJJ2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEC64ZAA#issuecomment-549309568>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AACHF6B3TLBCYRFOLV4JC63QR772LANCNFSM4GMYHJJQ> . |
The tkagg build doesn't interact with tk at build time whatsoever since#6442. Quoting from that PR:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Seem reasonable. Should we want optional Agg at some point, we can always reintroduce it. For now this is a simplification of the setup code.
Uh oh!
There was an error while loading.Please reload this page.
PR Summary
They are actually not optional (that's the
force = Truesetting whichoverrides the OptionalBackendPackage logic), so just make them
SetupPackages and remove the corresponding (outdated) comments in
setup.cfg.template.
Builds on top of#13074 as the prose in setup.cfg.template is only true if we also remove windowing from there. [edit: that PR has been merged]
(The OptionalBackendPackage logic was also previously useful back when the default backend was selected at build time, but that's no longer the case.)
PR Checklist