cucapra / pollen

generating hardware accelerators for pangenomic graph queries
MIT License
27 stars 1 forks source link

Expose slow_odgi's normalization in a new subcommand #108

Closed sampsyo closed 1 year ago

sampsyo commented 1 year ago

This solves a minor Python-related problem I ran into when running the tests on my machine: the python that the Turnt commands were using was the wrong one for the slow_odgi I had installed. The current Turnt config uses python -m slow_odgi.<stuff> in a few places, as opposed to a plain slow_odgi command, and the most prominent one is for normalization (slow_odgi.norm). This just works around the need for that by making slow_odgi norm into a proper subcommand.

To make this work, I also made all slow_odgi subcommands accept GFA files on stdin—in addition to the way they previously worked, which was to take a filename argument on the command line. Just leave off the filename to read from stdin instead.

I might try to make the other various python -m slow_odgi.<stuff> things into subcommands too at some point.

anshumanmohan commented 1 year ago

Thanks a bunch, Adrian! These changes look good, and I just did a little more to make our CI happy.

I think they some things were in the -m style because they (i.e. norm, paths, and the two setup commands) were all mini/meta commands that I did not want to give full command status. At the time I did not know that paths was an odgi command; I just needed it for setup purposes.

Since then, two things have happened:

Indeed, these are both documented in the slow_odgi README too.

A further little PR to make the calls to paths consistent would be super. Right now I use python -m slow_odgi.paths when I want paths as a setup routine, and slow_odgi paths when I want to test paths itself. Very silly.

I'm not sure we'd like to lift the two setup metacommands into their own commands. I don't feel strongly either way, but I will note that they should not really be called by users.

sampsyo commented 1 year ago

Awesome; thanks for the Black fix!

And yeah, if I do end up making those other utilities into subcommands, they won't be for end-user consumption…