-
Notifications
You must be signed in to change notification settings - Fork 94
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
cd to workflow dir during file parsing. #5589
Conversation
@@ -401,6 +401,12 @@ def read_and_proc( | |||
fpath = _get_fpath_for_source(fpath, opts) | |||
fdir = os.path.dirname(fpath) | |||
|
|||
odir = os.getcwd() |
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.
Might be safer
try:
os.chdir(fdir)
finally:
os.chdir(odir)
or even:
@contextmanager
def work_in(path):
before = os.getcwd()
try:
os.chdir(path)
finally:
os.chdir(before)
with work_in(fdir):
# ... stuff ...
Kind of surprised that the latter isn't part of Pathlib.
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.
It doesn't actually matter if we don't os.chdir
back afterwards, this only applies to the Python session itself not the parent shell.
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.
Note my change does os.chdir
back afterwards. I agree it should not matter, but to explicitly avoid any side-effects of the dir change that I might not be aware of.
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.
- Read code
- Tried code
I'm generally happy, but I've made some suggestions about the test.
4b8841b
to
54ee8fa
Compare
@wxtim - I tweaked the new test as you suggested. A final commit just cleans up the file to pass flake8. |
I'll hit merge as soon as the tests finish./ |
Allow the template processor to access workflow files - in the source dir for a source workflow, or the run dir for installed workflows.
This is the most straightforward way to cater to this kind of use case
(I can't think of any downside...)
Check List
CONTRIBUTING.md
and added my name as a Code Contributor.setup.cfg
(andconda-environment.yml
if present).CHANGES.md
entry included if this is a change that can affect users?.?.x
branch.