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

Fix bug with multiple regions in PCA and snp_allele_counts #338

Merged
merged 3 commits into from
Feb 23, 2023

Conversation

alimanfoo
Copy link
Member

@alimanfoo alimanfoo commented Feb 22, 2023

Comment on lines +3 to +12
# whitespace before ':' - doesn't work well with black
E203
# module level import not at top of file
E402
# line too long - let black worry about that
E501
# do not assign a lambda expression, use a def
E731
# line break before binary operator
W503
Copy link
Member Author

Choose a reason for hiding this comment

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

This is needed because of upgrading flake8 in pre-commit config.

Comment on lines 1 to 26
# https://pre-commit.com/
repos:
- repo: https://github.com/pre-commit/pre-commit-hooks
rev: v4.3.0
rev: v4.4.0
hooks:
- id: trailing-whitespace
- id: end-of-file-fixer
- id: check-yaml
# isort should run before black as black sometimes tweaks the isort output
- repo: https://github.com/pycqa/isort
rev: 5.10.1
rev: 5.12.0
hooks:
- id: isort
# https://github.com/python/black#version-control-integration
- repo: https://github.com/psf/black
rev: 22.10.0
rev: 23.1.0
hooks:
- id: black
- repo: https://github.com/keewis/blackdoc
rev: v0.3.8
hooks:
- id: blackdoc
- repo: https://github.com/pycqa/flake8
rev: 5.0.4
rev: 6.0.0
hooks:
- id: flake8
Copy link
Member Author

Choose a reason for hiding this comment

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

Made these upgrades because CI was failing with problems installing the pre-commit hooks.

@@ -4494,7 +4519,7 @@ def _read_cohort_metadata(self, *, sample_set):
},
inplace=True,
)
except FileNotFoundError:
except Exception:
Copy link
Member Author

Choose a reason for hiding this comment

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

This exception seems to have changed, needed to broaden.

@alimanfoo alimanfoo requested a review from leehart February 22, 2023 22:22
Copy link
Collaborator

@leehart leehart left a comment

Choose a reason for hiding this comment

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

LGTM.

Thanks for the helpful inline comments @alimanfoo . How do you do that?

@alimanfoo
Copy link
Member Author

Thanks for the helpful inline comments @alimanfoo . How do you do that?

No worries - I just go into the files tab and add single comments, a bit like doing a code review on my own PR.

@alimanfoo alimanfoo merged commit 873393f into malariagen:master Feb 23, 2023
@alimanfoo alimanfoo deleted the fix-pca-region-bug-2023-02-22 branch February 23, 2023 15:28
@alimanfoo alimanfoo added the BMGF-001927 Work supported by BMGF grant INV-001927 (MalariaGEN 2019-2024). label Dec 4, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
BMGF-001927 Work supported by BMGF grant INV-001927 (MalariaGEN 2019-2024).
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Anopheles PCA cannot handle list of regions Better caching for Ag3.pca()
2 participants