Fix Settings.pyenvs to _actually_ use basepath by default #344
Conversation
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
We have some test cases that verify correct interaction between --code/--deps and basepath. We're about to add --pyenv to --code and --deps as another option that interacts with basepath, and we'd like the tests to cover this as well. At this point the various combinations of passing or not passing these options on the command line are becoming tedious to hardcode. Instead, provide a `path_options` collection that list these options, and a subsets() helper function that automatically enumerates the various combinations of giving or not giving these options the we need in test_path_option_overrides_base_path().
This is a rather embarrasing omission from PR #326 (Teach FawltyDeps to automatically discover Python environments inside the project): Although that PR did include code to traverse within a given --pyenv path to find Python environments, the PR "forgot" to actually switch the _default_ behavior of --pyenv: The default value of settings.pyenvs remained an empty set, and there were no changes to have basepath influence the value of settings.pyenvs. This commit fixes that: - When neither --pyenv nor basepath is given, settings.pyenvs should default to the current directory. - When --pyenv is not given, but basepath is given, basepath should override the default settings.pyenvs. - When --pyenv is given, it overrides basepath. In short settings.pyenvs should behave exactly like .code and .deps.
Nour-Mws
approved these changes
Jul 31, 2023
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
This is a rather embarrasing omission from PR #326 (Teach FawltyDeps to automatically discover Python environments inside the project):
Although that PR did include code to traverse within a given
--pyenvpath to find Python environments, the PR "forgot" to actually switch the default behavior of--pyenv:The default value of
settings.pyenvsremained an empty set, and there were no changes to havebasepathinfluence the value ofsettings.pyenvs.This PR fixes that:
--pyenvnorbasepathis given,settings.pyenvsshould default to the current directory.--pyenvis not given, butbasepathis given,basepathshould override the defaultsettings.pyenvs.--pyenvis given, it overridesbasepath.In short
settings.pyenvsshould behave exactly like.codeand.deps.Commits:
test_settings: Prepare for including--pyenvinbasepath-related testsSettings.pyenvsto actually usebasepathby default