-
-
Notifications
You must be signed in to change notification settings - Fork 7.4k
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 our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Make optional backends respect setup.cfg #2331
Merged
mdboom
merged 7 commits into
matplotlib:v1.3.x
from
megies:optional_backend_config_check
Sep 9, 2013
Merged
Changes from 6 commits
Commits
Show all changes
7 commits
Select commit
Hold shift + click to select a range
f453421
make all optional backends respect the cfg file
megies 162cf12
cleaning up OptionalBackendPackage a tiny bit
megies fc759ee
move general check() method of optional backend to base class
megies db54c39
replace checking of configuration of optional backends in overridden …
megies a4ac003
refactoring/separating backend checks for config file/dependencies
megies d3ae52f
update list of gui backends in config template
megies 829597b
refactoring and unifying OptionalPackage and OptionalBackendPackage
megies File filter
Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
I think this message will be ultimately misleading. If
setup.cfg
isTrue
orauto
, but the dependency isn't found, it will read something likeinstalling, gtk not found
. I think in that case, the message from check_requirements should just override, but not append to,self.check
.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.
Hmm, I just looked at it again, and I am kind of arriving at the same logic again (which was there before I made changes, I really only refactored it into two routines):
Looking at
setup.py
where the package checks are called, any raisedCheckFailed
Exception drops out of the installation (ignoring any install message assigned in there like "installing") and its Exception message is used exclusively to indicate the reason. Only ifcheck_requirements()
properly returns (noraise
), this string is appended and the package is assigned for installing insetup.py
.Summary..
check()
raises -> "no [skipping due to configuration]"check_requirements()
raises -> "no [exception msg as defined in overriddencheck_requirements()
]"check_requirements()
returns string -> "yes ['installing' + ', ' + custom message bycheck_requirements()
]"check_requirements()
returns string -> "yes ['installing forced (config override)' + ', ' + custom message bycheck_requirements()
]"Let me know if I am missing something here..