DM-39885: Fully substitute symbolic environment variables in symbolic filenames #17
Conversation
python/lsst/ctrl/bps/parsl/job.py
Outdated
| # symbolic names. As a result, more than one iteration may be required | ||
| # to resolve all symbolic names. For example, an actual value for | ||
| # a filename may contain a symbolic name for an environment variable. | ||
| while re.search(_env_regex, command) or re.search(_file_regex, command): |
There was a problem hiding this comment.
Just a thought, but instead of repeating the search here and in the re.sub lines, maybe another option for the while is to loop until command stops changing? ie something like (untested):
prev_command = command
while True:
command = re.sub(...)
command = re.sub(...)
if command == prev_command:
break
prev_command = commandThere may be a neater way to organize things but it seems better than running the regexes twice as often.
There was a problem hiding this comment.
Sure, I could use this version instead.
BTW, just out of curiosity, I timed both version with timeit. For a fairly short string like for example
<ENV:CTRL_MPEXEC_DIR>/bin/pipetask --long-log --log-level=VERBOSE pre-exec-init-qbb <FILE:butlerConfig> <FILE:runQgraphFile>
your version is ~10% faster than mine, i.e., it took 11.41 s to complete 1,000,000 executions for my version and 10.31 s for yours.
There was a problem hiding this comment.
So most of the overhead is in the re.sub. I do think that "no string was replaced" is a neater end state since that's what you are actually wanting.
|
I have fixes for the docstyle and build_sphinx_docs failures on #16. The build failure is caused by pydantic 2.0 being pulled in from the pypi daf_butler. This should work fine once a new PyPI release is made tonight. |
timj
changed the title
Codecov ReportPatch and project coverage have no change.
Additional details and impacted files@@ Coverage Diff @@
## main #17 +/- ##
=======================================
Coverage 69.23% 69.23%
=======================================
Files 3 3
Lines 26 26
Branches 6 6
=======================================
Hits 18 18
Misses 6 6
Partials 2 2 ☔ View full report in Codecov by Sentry. |
Checklist
doc/changes