merge LinkFinder to MetcalfScoring class #248
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This was referenced May 31, 2024
Merged
Member
Author
This was referenced May 31, 2024
This was referenced May 31, 2024
gcroci2
approved these changes
Jun 4, 2024
| assert isinstance(mc.metcalf_mean, np.ndarray) | ||
| assert isinstance(mc.metcalf_std, np.ndarray) | ||
| assert mc.metcalf_mean.shape == (4, 4) # (n_strains+1 , n_strains+1) | ||
| assert mc.metcalf_mean.shape == (4, 4) |
Contributor
There was a problem hiding this comment.
Suggested change
| assert mc.metcalf_mean.shape == (4, 4) |
This is repeated twice, maybe you wanted to assert metcalf_std shape?
Member
Author
There was a problem hiding this comment.
Good catch! Indeed, it should be metcalf_std.
| assert isinstance(mc.metcalf_mean, np.ndarray) | ||
| assert isinstance(mc.metcalf_std, np.ndarray) | ||
| assert mc.metcalf_mean.shape == (4, 4) # (n_strains+1 , n_strains+1) | ||
| assert mc.metcalf_mean.shape == (4, 4) |
Contributor
There was a problem hiding this comment.
Same as above, this is repeated twice
| assert isinstance(mc.metcalf_mean, np.ndarray) | ||
| assert isinstance(mc.metcalf_std, np.ndarray) | ||
| assert mc.metcalf_mean.shape == (4, 4) # (n_strains+1 , n_strains+1) | ||
| assert mc.metcalf_mean.shape == (4, 4) |
Contributor
There was a problem hiding this comment.
Same as above, this is repeated twice
3f8bbdd to
094203b
Compare
3a92298 to
89aa35e
Compare
094203b to
9bd9d1a
Compare
89aa35e to
145bd22
Compare
Member
Author
Merge activity
|
The actual role of LinkFinder is to calculate metcalf score, so it makes more sense to merge its functions to MetcalfScoring class
caplog cannot capture all logs, so remove the assertions.
This option is much faster and easier to control the group of tests for same worker.
3974c23 to
d13d883
Compare
This was referenced Jun 7, 2024
Merged
Merged
This was referenced Jun 17, 2024
Merged
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The actual role of
LinkFinderis to calculate and store metcalf scores. This PR merges these roles toMetcalfScoringclass, which will increase the clarity of the scoring process.Major changes:
To make unit tests work:
remove
unit/conftest.pythat has one temporary root dir for all tests. Now you have to create temporary root dir only in the places that need it, which makes unit tests indepdent with each other in the parallel testing.change pytest xdist from
--dist loadscopeto--dist loadgroup, which is much faster for parallel testing and easier to control the group of tests for same worker..