GenomicMedLab / cool-seq-tool

https://coolseqtool.readthedocs.io
MIT License
4 stars 0 forks source link

feat!: use setuptools-scm, capture version at package root #301

Closed jsstevenson closed 2 months ago

jsstevenson commented 2 months ago
jsstevenson commented 2 months ago

currently the version value gets used in 4 places within the library:

1) the API routes 2) exon-genomic coords responses 3) the pydantic models (exemplifying the API routes) 4) the docs

We've talked about removing the API routes entirely. The exon/genomic response seems a little unnatural, I feel like it might be better to let the caller decide to timestamp it themselves (and they can also use importlib.metadata to capture CST versioning if they want).

I wonder if we could simplify things by avoiding any version variable within the library code itself, which means we don't need a version.py file or a variable in __init__.py. At the very least, if/until we remove the API routes, we could move the version capturing up to that module instead of having it in the greater library code.


todo: figure out whether we're ok on the readthedocs check failure

jsstevenson commented 2 months ago

todo: figure out whether we're ok on the readthedocs check failure

Should work once we tag a new release with a legal version scheme. Won't work until then.

korikuzma commented 2 months ago

We've talked about removing the API routes entirely

I think we were waiting on @jarbesfeld to confirm with folks on this.

jsstevenson commented 2 months ago

@korikuzma I'd like to put out a CST release, before this is merged, with a compatible version (either 0.5.0 or 0.4.1), just to verify that readthedocs will pass

jsstevenson commented 2 months ago

checks are passing now that the base version is a legal version value for setuptools-scm

currently the version value gets used in 4 places within the library:

the API routes
exon-genomic coords responses
the pydantic models (exemplifying the API routes)
the docs

We've talked about removing the API routes entirely. The exon/genomic response seems a little unnatural, I feel like it might be better to let the caller decide to timestamp it themselves (and they can also use importlib.metadata to capture CST versioning if they want).

I wonder if we could simplify things by avoiding any version variable within the library code itself, which means we don't need a version.py file or a variable in init.py. At the very least, if/until we remove the API routes, we could move the version capturing up to that module instead of having it in the greater library code.

I thought about it more and I think getting rid of __version__ entirely is unnecessary, though I do think we should look at a little more of this code pruning soon