Skip to content

Is the path variable still a thing? #322

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

Closed
nsheff opened this issue Mar 26, 2021 · 8 comments
Closed

Is the path variable still a thing? #322

nsheff opened this issue Mar 26, 2021 · 8 comments

Comments

@nsheff
Copy link
Contributor

nsheff commented Mar 26, 2021

Here, we introduce the path key in the pipeline interface.

http://looper.databio.org/en/latest/writing-a-pipeline-interface/

image

Are these docs outdated? I seem to recall that we switched from this to use var_templates

There are very few docs related to this and I think some of these are wrong...

@nsheff nsheff added the docs label Mar 26, 2021
@aaron-gu
Copy link
Contributor

aaron-gu commented May 6, 2021

This is working for me in the hello looper repository as well as a project I created separately. (looper v1.3.0)

@donaldcampbelljr
Copy link
Contributor

Currently, in v1.4.0, one can replace
path: count_lines.sh

with

image

or use:
image

However, one page of our documentation instructs to use:
image

image

However, this does not execute the pipeline script as expected. Looper debug output:

`DEBU 14:11:53 | looper.utils:utils:207 > rendered arg str: {looper.piface_dir}/pipeline/count_lines.sh data/frog2_data.txt --output-parent /home/drc/hello_looper_results/results_pipeline/frog_2``

vs executing the pipeline .sh as expected:

looper.utils:utils:207 > rendered arg str: pipeline/count_lines.sh data/frog2_data.txt

It appears that in the 1st case, rendered arg str: shows {looper.piface_dir} is not being replaced with the correct file path to call the pipeline script.

Also, using the example from the documentation page, namespace pipelines also shows incorrect path, e.g. pipeline/pipeline/count_lines.sh vs pipeline/count_lines.sh

@donaldcampbelljr
Copy link
Contributor

Downgrading to Looper 1.3.2 and using Python 3.8:

Still not calling the script due to an 'extra' "pipeline" in the called path:
image

image

However, if one uses {looper.piface_dir}/count_lines.sh vs {looper.piface_dir}/pipeline/count_lines.sh, it will call the script properly:
image

image

Submission for an example sample using Looper 1.3.2:
image

Submission for an example sample using the same config files but using Looper 1.4.0:
image

@donaldcampbelljr
Copy link
Contributor

Additional information, confirmed that:

Command template arguments that also contain {} will not parse correctly, e.g. {pipeline.var_templates.pipeline} ------> points to ------> '{looper.piface_dir}/count_lines.sh'

Not rendered appropriately:
Screenshot from 2023-04-05 11-25-47

Changing the config file to have all '{}' commands within command template will rendered the proper script path and execute the pipeline as expected:
Screenshot from 2023-04-05 11-37-36

I believe this occurs in function: def jinja_render_template_strictly.

@donaldcampbelljr
Copy link
Contributor

Added check in utils.py to look for nested commands: f3ae33f

Next Steps:

  1. Update Looper Documentation to show examples using var_templates/pipeline: https://looper.databio.org/en/latest/writing-a-pipeline-interface/
  2. Note that path variable will be deprecated.
  3. Update hello_world repository to include var_templates (instead of simple path variable).

@donaldcampbelljr
Copy link
Contributor

Per discussion, a different approach for the nested commands was investigated. Instead of checking for nested commands, the program can return the rendered 'var_templates' back to the 'pipeline' namespace.

Proposed change in conductor.py, after rendering var_templates:

            pl_iface[VAR_TEMPL_KEY] = self.pl_iface.render_var_templates(
                namespaces=namespaces
            )
            _LOGGER.debug(f"namespace pipelines: { pl_iface }")

            namespaces["pipeline"]["var_templates"] = pl_iface[VAR_TEMPL_KEY]

This will allow the the proper rendering of the following command_template as it uses the pipeline namespace for rendering NOT pl_iface:


pipeline_name: count_lines
pipeline_type: sample
output_schema: output_schema.yaml
var_templates:
  pipeline: '{looper.piface_dir}/count_lines.sh'
command_template: >
  {pipeline.var_templates.pipeline} {sample.file} --output-parent `{looper.sample_output_folder}

@donaldcampbelljr
Copy link
Contributor

This is now fixed with 79fef3e

@nsheff
Copy link
Contributor Author

nsheff commented Apr 13, 2023

excellent -- it would probably be best in this case to add add one small pytest to check for this so we don't have a reversion

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants