Skip to content

Conversation

@dlaehnemann
Copy link
Contributor

This pull request also standardizes the script argument parsing and invocation setup.

In more detail, for automatically installing the scripts, this pull request does the following:

  1. Creates new cli submodules under both the skgenome (with skg_convert.py) and cnvlib (all the other python scripts) modules, both with empty __init__.py files.
  2. Under [tool.setuptools] in the pyproject.toml, also list those new submodules as "cnvlib.cli" and "skgenome.cli". I just did this by analogy to the existing entries and because otherwise the cli modules would not be found in the installation. Not sure if this is the correct / best way?
  3. Under [project.scripts] in the pyproject.toml, create an entry_point for each of the scripts.

Further, the pull reqiest does the following to standardize the scripts:

  1. Consistently moves argument parsing into an argument_parsing() function that takes no arguments.
  2. Consistently moves the actual code for running the script into a function that has the same name that the script file has. This always takes the args produced by the argument_parsing() function as input.
  3. Invoke both the argument parsing and the script function in a main() function that does not take any arguments.
  4. Under if __name__ == '__main__': (so when calling this as a script), they just call the main() function.

With this new structure, any functions used in these scripts could be reused in other code parts by importing them from the newly created modules. I guess this is unlikely, but now it has a consistent structure for it.

I tested each of the scripts after a pip-based installation, and they all properly run with --help. Also, for all of the scripts (except cnv_expression_correlate.py) I quickly found some example data in the test/ folder or from my projects, and can confirm that they run through from such an installation. So I think that all the importing updates are also correct.

If there is some further testing infrastructure for the scripts that I have missed, please let me know. And code reviews and more testing from others is of course also welcome!

Also, I might pursue adding this pull request as a patch to the bioconda recipe until this is merged and released. Because what I am really aiming for here is to have the script installation automatable via conda for a standardized snakemake workflow.

…nstallation, standardise script argument parsing and invocation setup
@etal
Copy link
Owner

etal commented Jul 25, 2025

Thanks for putting in this work. Let me take a closer look at this one.

@dlaehnemann
Copy link
Contributor Author

It has been working for us with skgconvert.py in the bioconda recipe since the patch with these exact changes was applied here:
bioconda/bioconda-recipes#57334

So you could quickly test this out with a conda installation with:

conda create -n cnvkit -c conda-forge -c bioconda cnvkit

Let me know if you have any further questions regarding this.

@etal etal merged commit 0478fa6 into etal:master Aug 8, 2025
@etal
Copy link
Owner

etal commented Aug 8, 2025

Wonderful, thanks!

@dlaehnemann dlaehnemann deleted the feat/bring-back-scripts-installation branch August 18, 2025 09:26
@dlaehnemann
Copy link
Contributor Author

Let me know when you create a new release, so I can follow up on the bioconda recipe. Currently, the change here is applied as a patch there, and that won't be necessary (and won't work) any more.

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.

2 participants