jjmccollum / teiphy

A Python package for converting TEI XML collations to NEXUS, BEAST 2.7 XML, and other formats
MIT License
11 stars 3 forks source link

Add --version argument to app #47

Closed jjmccollum closed 1 year ago

jjmccollum commented 1 year ago

Per the first round of reviewer comments on the paper (https://github.com/openjournals/joss-reviews/issues/4879), it would be convenient to support a --version argument for the command-line interface that returns the current version of the code.

This should be straightforward, but in the interest of preventing oversight in future updates, it would be nice if we only had to update the version number of the project in one place (specifically, in pyproject.toml). @rbturnbull, do you know if there's a preferred way of reading a package's version number within Python for this purpose? I'm seeing several suggestions in the thread at https://github.com/python-poetry/poetry/issues/273, so I wanted to see how you approach this problem.

I've opened a code-revision branch where we can make this change.

rbturnbull commented 1 year ago

I think we should be able to do it with a callback in typer and using importlib_metadata. There is a way to do it with a standard python module but I think that's only from a recent python release. Have a look here: https://github.com/rbturnbull/ausdex/blob/main/ausdex/main.py

rbturnbull commented 1 year ago

also here: https://github.com/rbturnbull/torchapp/blob/master/torchapp/apps.py

jjmccollum commented 1 year ago

Thanks! I tried adapting the code in the first example, as it seemed cleaner. I added from importlib.metadata import version to the preamble. Then I added the following function:

def version_callback(value: bool):
    if value:
       teiphy_version = version("teiphy")
        typer.echo(teiphy_version)
        raise typer.Exit()

And I added the following option to the app:

version: bool = typer.Option(
    False,
    callback=version_callback,
    is_eager=True,
    help="Print the current version.",
),

Entering the command poetry run teiphy --version works as expected, but the program keeps printing the version number "0.1.0" to the console. The version number in pyproject.toml is "0.1.3", so now I'm trying to figure out what is causing the discrepancy. I cleaned out the dist folder and ran poetry build just to be safe, but I'm still getting a version number of "0.1.0". I've pushed my changes to the code-revision branch, if you'd like to test this out for yourself.

rbturnbull commented 1 year ago

I'll check it out. By the way - what version of poetry are you using? I had a similar problem to this in another project before upgrading to poetry 2

jjmccollum commented 1 year ago

Ah, perhaps that's the issue; I'm using version 1.1.13. I can try upgrading now.

If poetry 2 is necessary, is there a line in the pyproject.toml file that we should change accordingly? The following lines look like they might fit:

[build-system]
requires = ["poetry-core>=1.0.0"]
rbturnbull commented 1 year ago

I think it's just that you need to clear out the old distribution files before the right version number is picked up. I don't think we need to require poetry 2 in pyproject.toml. Up to you...

jjmccollum commented 1 year ago

Is there anything I need to clear out apart from the files in the dist directory? I've done that and run poetry build, but I still get the wrong version number.

rbturnbull commented 1 year ago

it's working for me on the code-revision branch.

Try clearing out these directories in your virtual env: .venv/lib/python3.9/site-packages/teiphy-* (wherever poetry installs your venv)

Then try poetry install

jjmccollum commented 1 year ago

That did it! Thanks for suggesting clearing out the cache for the virtual environment.

Since we've established that this feature works, I'll go ahead and close this issue. I'm almost done implementing output methods for the PHYLIP and FASTA formats, but I'll have to add unit tests to ensure 100% coverage, and if it's easy to enough to add testing workflows for PHYLIP and/or RAxML, then we may want to do that. Once all of that is in place, we can merge code-revision into main.

rbturnbull commented 1 year ago

sounds great