Conversation
BrunoSanchez
force-pushed
the
tickets/DM-48896
branch
3 times, most recently
from
47989eb to
c47b45a
Compare
BrunoSanchez
force-pushed
the
tickets/DM-48896
branch
from
787e271 to
dd43cfa
Compare
.gitignore
Outdated
|
|
||
| # Dynamically generated source injection pipelines | ||
| pipelines/_ingredients/ApPipeWithFakes.yaml | ||
| python/*.dist-info/ No newline at end of file |
SConstruct
Outdated
| from lsst.sconsUtils import scripts | ||
| scripts.BasicSConstruct("ap_pipe", disableCc=True) | ||
| from lsst.sconsUtils.state import env | ||
| from lsst.sconsUtils import targets |
There was a problem hiding this comment.
scripts, targets merged into above.
SConstruct
Outdated
| ) | ||
|
|
||
| subset_names = "apPipe,prompt" | ||
| subset_description = "Post injection diffim analysis tasks" |
There was a problem hiding this comment.
This isn't being used right now, but may become irrelevant following a proposed API change in source_injection.
BrunoSanchez
force-pushed
the
tickets/DM-48896
branch
from
dd43cfa to
d67b5c0
Compare
BrunoSanchez
force-pushed
the
tickets/DM-48896
branch
3 times, most recently
from
d2d6c6f to
379ef9f
Compare
SConstruct
Outdated
| [ | ||
| libraryLoaderEnvironment(), | ||
| f"make_injection_pipeline -t preliminary_visit_image -r $SOURCE -f $TARGET " | ||
| f"-a {additional_fakes_tasks} -s "+" -s ".join(subset_names), |
There was a problem hiding this comment.
Should -s be used twice here?
There was a problem hiding this comment.
It is not, it is rendered correctly, since the .join string method uses the string instance as an intermediate link for the iterable elements. If there is no "first" -s then the .join method doesn't provide it and the command fails.
There was a problem hiding this comment.
And if subset_names is length zero, this still prints a single -s then? Is that an issue?
There was a problem hiding this comment.
I think it would add an empty string, but since this is a script, and the previous instruction defines subset_names to not be zero, then I assumed it was OK to add it this way. If we need to make it more robust I can change it though we will have to include a command building instruction before this, so we can take all cases into consideration.
There was a problem hiding this comment.
I agree that the hard-coding of subset_names above will always resolve this, but it may read a little confusingly to the casual observer. How about this instead?
" ".join(f"-s {subset_name}" for subset_name in subset_names)
leeskelvin
left a comment
leeskelvin
left a comment
There was a problem hiding this comment.
One question inside the SConstruct, but otherwise LGTM!
BrunoSanchez
force-pushed
the
tickets/DM-48896
branch
from
26db4fa to
9a84c69
Compare
Add yaml to process LSSTComCam with fakes
Improve command for ApPipeWithFakes pipeline creation
BrunoSanchez
force-pushed
the
tickets/DM-48896
branch
from
9a84c69 to
d195e26
Compare
2 participants
This has evolved and now it is scoped for replacing all pipeline creation