Repository: scikit-learn/scikit-learn
Pull request: [MGE] MAINT Adds cwd to PYTHONPATH in assert_run_python_script
<!--
Thanks for contributing a pull request! Please ensure you have taken a look at
the contribution guidelines: https://github.com/scikit-learn/scikit-learn/blob/master/CONTRIBUTING.md#pull-request-checklist
-->

#### Reference Issues/PRs
<!--
Example: Fixes #1234. See also #3456.
Please use keywords (e.g., Fixes) to create link to the issues or pull requests
you resolved, so that they will automatically be closed when your pull request
is merged. See https://github.com/blog/1506-closing-issues-via-pull-requests
-->
Fixes https://github.com/scikit-learn/scikit-learn/issues/14418


#### What does this implement/fix? Explain your changes.
Adds cwd to PYTHONPATH in assert_run_python_script

<!--
Please be aware that we are a loose team of volunteers so patience is
necessary; assistance handling other issues is very welcome. We value
all user contributions, no matter how minor they are. If we are slow to
review, either the pull request needs some benchmarking, tinkering,
convincing, etc. or more likely the reviewers are simply busy. In either
case, we ask for your understanding during the review process.
For more information, see our FAQ on this topic:
http://scikit-learn.org/dev/faq.html#why-is-my-pull-request-not-getting-any-attention.

Thanks for contributing!
-->


Problem statement:
test_imports_strategies fails unless sklearn is installed
So I ran make in a fresh environment and ``test_imports_strategies`` fails because it doesn't find sklearn.

I'm not entirely sure if this should be considered a bug, but previously all tests worked without installing sklearn.

Hints from issue discussion:

Commit message: MAINT Adds shell=True

Patch to review:
diff --git a/sklearn/utils/testing.py b/sklearn/utils/testing.py
index 8913d5c92e0ca..3660aa1a279a7 100644
--- a/sklearn/utils/testing.py
+++ b/sklearn/utils/testing.py
@@ -881,6 +881,7 @@ def assert_run_python_script(source_code, timeout=60):
             'cwd': cwd,
             'stderr': STDOUT,
             'env': env,
+            'shell': True
         }
         # If coverage is running, pass the config file to the subprocess
         coverage_rc = os.environ.get("COVERAGE_PROCESS_START")


Code review comments:
diff_hunk: @@ -881,6 +881,7 @@ def assert_run_python_script(source_code, timeout=60):
             'cwd': cwd,
             'stderr': STDOUT,
             'env': env,
+            'shell': True
path: sklearn/utils/testing.py
text: @user1:
As far as I know `shell=True` should be avoided. See https://stackoverflow.com/a/3172488/1791279

Why do you think that `cwd` doesn't work without it? I couldn't find anything about it in the docs.

@author:
Ah it is working correctly. Changing working directly does not automatically add that path to the `PYTHONPATH`. This PR was updated to add cwd to `PYTHONPATH`.