Skip to content

Conversation

@petrelharp
Copy link
Contributor

Closes #821, although not in the most elegant way.

@petrelharp petrelharp force-pushed the mutation_row_equality branch from 2cc1712 to 2c6209d Compare August 31, 2020 17:51
@petrelharp
Copy link
Contributor Author

Also a small clarifying note about pre-commit.

... which is not working for me, for some reason: running pre-commit run does everything as expected, but then git commit fails with a bunch of flake8 errors beginning with

python/tskit/metadata.py:161:13: F541 f-string is missing placeholders
python/tskit/util.py:111:26: E203 whitespace before ':'
python/tskit/util.py:129:31: E203 whitespace before ':'
python/tskit/util.py:189:23: E203 whitespace before ':'
python/tskit/util.py:207:36: E203 whitespace before ':'

The only way to get it to commit is to use --no-verify. I've uninstalled/reinstalled the pre-commit hooks, my flake8 is up to date, I've looked for other hooks... no idea what's going on. Any ideas?

Copy link
Member

@jeromekelleher jeromekelleher 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 @petrelharp. I think we should wait on @benjeffery before merging though (he'll be back in a few days).

time: float

def __eq__(self, other):
return (
Copy link
Member

Choose a reason for hiding this comment

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

Looks like you need an isinstance(other, MutationTableRow) here and return False otherwise.

@benjeffery
Copy link
Member

Opps I meant to comment not approve - just that one fix to the eq method.

Thanks for finding and fixing my omission @petrelharp!

@petrelharp petrelharp force-pushed the mutation_row_equality branch 3 times, most recently from 4f2cf14 to b436d2b Compare September 1, 2020 15:17
@petrelharp
Copy link
Contributor Author

Thanks for finding and fixing my omission @petrelharp!

It's /our/ omission, isn't it? But, thanks for getting everything so wonderfully in line that the dangling ends are so minor!

@petrelharp
Copy link
Contributor Author

I've added more tests. I think this is good to go.

Copy link
Member

@jeromekelleher jeromekelleher left a comment

Choose a reason for hiding this comment

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

LGTM. I'll let @benjeffery push the green button though, in case he sees anything.

@codecov
Copy link

codecov bot commented Sep 1, 2020

Codecov Report

Merging #822 into master will increase coverage by 0.00%.
The diff coverage is 100.00%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #822   +/-   ##
=======================================
  Coverage   93.57%   93.57%           
=======================================
  Files          24       24           
  Lines       19545    19547    +2     
  Branches      789      789           
=======================================
+ Hits        18290    18292    +2     
  Misses       1223     1223           
  Partials       32       32           
Impacted Files Coverage Δ
python/tskit/tables.py 99.70% <100.00%> (+<0.01%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 1c0f04f...4be313c. Read the comment docs.

@mergify mergify bot merged commit dcdd12a into tskit-dev:master Sep 1, 2020
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.

mutation row with missing time not equal to itself

3 participants