Skip to content
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

build: migrate from setup.py to pyproject.toml and set up CI #193

Merged
merged 50 commits into from
Nov 8, 2023

Conversation

aryarm
Copy link
Member

@aryarm aryarm commented Oct 30, 2023

...since setup.py is obsolete (source)

we also migrate from travis CI and code-cov to github actions according to these directions. This will allow us to integrate testing into the PR approval process

Note: We still don't have a lock file or any version constraints in our pyproject.toml, so our CI is still prone to death whenever a breaking change to one of our dependencies is uploaded to PyPI. If we want to tackle that, it can be a future-me problem 😅

note: this pr also fixes some issues arising from deprecations in the readthedocs config (source) and resolves all warning messages in the current readthedocs build. In order to get the tests to pass, I also had to make some small alterations to the code

@aryarm
Copy link
Member Author

aryarm commented Oct 30, 2023

@LiterallyUniqueLogin, can I ask you for approval for this, too?

it should also help to fix the failing build issues in @aarushi03's PR (#191)

@aryarm aryarm changed the base branch from develop to master October 30, 2023 21:25
ERROR trtools/testsupport/sample_vcfs/associaTR/generate_traits.py - FileNotFoundError: [Errno 2] No such file or directory: 'samples.txt'
@aryarm aryarm changed the title build: migrate from setup.py to pyproject.toml build: migrate from setup.py to pyproject.toml and add CI Oct 30, 2023
ERROR trtools/testsupport/sample_vcfs/associaTR/make_dosages.py - OSError: Error opening many_samples_biallelic.vcf.gz
@aryarm
Copy link
Member Author

aryarm commented Oct 30, 2023

@LiterallyUniqueLogin, it looks like the tests are failing because bgzip, bcftools, and tabix aren't installed

ERROR trtools/testsupport/sample_vcfs/associaTR/make_dosages.py - subprocess.CalledProcessError: Command 'bash -c "bgzip -f gp_dosages.tsv && bgzip -f ap1_dosages.tsv && bgzip -f ap2_dosages.tsv && bcftools annotate -a gp_dosages.tsv.gz -h gp_dosage_header.hdr -S <(tail -n +2 samples.txt) -c CHROM,FROM,TO,FMT/GP many_samples_biallelic.vcf.gz | bcftools annotate -a ap1_dosages.tsv.gz -h ap1_dosage_header.hdr -S <(tail -n +2 samples.txt) -c CHROM,FROM,TO,FMT/AP1 - | bcftools annotate -a ap2_dosages.tsv.gz -h ap2_dosage_header.hdr -S <(tail -n +2 samples.txt) -c CHROM,FROM,TO,FMT/AP2 - > many_samples_biallelic_dosages.vcf && bgzip -f many_samples_biallelic_dosages.vcf && tabix -f many_samples_biallelic_dosages.vcf.gz "' returned non-zero exit status 127.

Are those tools necessary for the associaTR tests? If so, I could try to install them in the CI environment.
But if it turns out to be too difficult to install, would it be ok if we just skipped whichever tests require those tools?

@aryarm aryarm marked this pull request as draft October 31, 2023 04:45
@aryarm aryarm changed the title build: migrate from setup.py to pyproject.toml and add CI build: migrate from setup.py to pyproject.toml and set up CI Oct 31, 2023
Copy link
Contributor

@LiterallyUniqueLogin LiterallyUniqueLogin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looking good overall! I haven't tested that any of the changes work as described, I've just read through the docs. Just a few minor comments.

Thank you for all the work to keep this package maintained!

trtools/__init__.py Show resolved Hide resolved
pyproject.toml Outdated Show resolved Hide resolved
pyproject.toml Outdated Show resolved Hide resolved
pyproject.toml Outdated Show resolved Hide resolved
PUBLISHING.rst Outdated Show resolved Hide resolved
PUBLISHING.rst Show resolved Hide resolved
Copy link
Contributor

@LiterallyUniqueLogin LiterallyUniqueLogin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All looking good. One last change: I'd remove the mention of using the web interface to create tags under the section "git tagging" (and then also remove the mention that there are two ways). Reason being that it seems this process requires the tag to exist locally so that python -m build uses that tag, and the web interface wouldn't meet that requirement, right?

@aryarm
Copy link
Member Author

aryarm commented Nov 8, 2023

Actually, if they use the web interface, the tag should still exist locally as long as they do a git pull, which is already one of the steps anyway. I'll add a message reiterating that the most recent commit needs to be tagged - just in case that helps.
Eventually, it will be good to automate the entire release process one day, anyway.

@LiterallyUniqueLogin LiterallyUniqueLogin merged commit d815f76 into master Nov 8, 2023
4 checks passed
@aryarm aryarm deleted the build/pyproject branch November 8, 2023 23:52
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