Skip to content
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

BF: Runner should launch scripts from their own directory #2816 #2819

Merged
merged 5 commits into from
Mar 5, 2020
Merged

BF: Runner should launch scripts from their own directory #2816 #2819

merged 5 commits into from
Mar 5, 2020

Conversation

jfkominsky
Copy link
Contributor

Attempted fix for #2816

The new runner changed how the cwd was set so that it no longer used the file's directory as the working directory, and instead defaulted to the app folder. This fix uses the fileName string and uses rfind to locate the last instance of a folder delimiter ('\' for windows, '/' for everything else) to set the working directory to the path of the current file. Tested on Mac, not yet tested in Windows.

Updating to latest version for bug fix
updating to current, prep to fix runner cwd issue
See issue #2186. The new runner changed how the cwd was set so that it no longer used the file's directory as the working directory, and instead defaulted to the app folder. This fix uses the fileName string and uses rfind to locate the last instance of a folder delimiter ('\\' for windows, '/' for everything else) to set the working directory to the path of the current file. Tested on Mac, not yet tested in Windows.
@coveralls
Copy link

Coverage Status

Coverage decreased (-0.008%) to 48.645% when pulling 1a82ee7 on jfkominsky:master into 3e8adf5 on psychopy:master.

@coveralls
Copy link

coveralls commented Feb 29, 2020

Coverage Status

Coverage increased (+0.007%) to 48.68% when pulling 0956a8f on jfkominsky:master into 55e103a on psychopy:master.

@codecov-io
Copy link

codecov-io commented Feb 29, 2020

Codecov Report

Merging #2819 into master will increase coverage by 0.01%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #2819      +/-   ##
==========================================
+ Coverage   44.43%   44.45%   +0.01%     
==========================================
  Files         224      242      +18     
  Lines       38928    45032    +6104     
  Branches     6730     7650     +920     
==========================================
+ Hits        17298    20018    +2720     
- Misses      19832    22953    +3121     
- Partials     1798     2061     +263
Impacted Files Coverage Δ
psychopy/experiment/flow.py 59.52% <0%> (-14.78%) ⬇️
psychopy/app/builder/dialogs/dlgsCode.py 59.66% <0%> (-12.82%) ⬇️
psychopy/visual/backends/gamma.py 20.93% <0%> (-10.82%) ⬇️
psychopy/visual/rift.py 2.48% <0%> (-9.39%) ⬇️
psychopy/experiment/components/code/__init__.py 62.19% <0%> (-8.23%) ⬇️
psychopy/app/pavlovia_ui/toolbar.py 67.92% <0%> (-6.59%) ⬇️
psychopy/visual/rect.py 88.57% <0%> (-5.55%) ⬇️
psychopy/visual/window.py 62.9% <0%> (-4.94%) ⬇️
...hopy/experiment/components/parallelOut/__init__.py 77.04% <0%> (-3.99%) ⬇️
psychopy/app/icons.py 77.41% <0%> (-3.23%) ⬇️
... and 138 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 5554c4a...1a82ee7. Read the comment docs.

Fixes CWD issue with PathLib instead of manual string editing.
@jfkominsky
Copy link
Contributor Author

Updated fix uses PathLib, and has been tested on Windows. Good to go.

@peircej peircej merged commit 0d661f7 into psychopy:master Mar 5, 2020
@peircej
Copy link
Member

peircej commented Mar 5, 2020

Thanks Jonathan. Oh, for commit messages try to prefx what type of change this is. BF indicates to me something that should be ported to bug-fix releases (we want this in 2020.1.3) whereas enhancements (ENH) and (new)FeatureFixes (FF) should be held back until 2020.2.0
https://www.psychopy.org/developers/repository.html#commit-messages
cheers!

@peircej peircej changed the title BF: #2816 BF: Runner should launch scripts from their own directory #2816 Mar 6, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants