Skip to content
This repository has been archived by the owner on Dec 7, 2018. It is now read-only.

Move sampling transforms to their own class #38

Merged
merged 8 commits into from
Jul 14, 2018
Merged

Move sampling transforms to their own class #38

merged 8 commits into from
Jul 14, 2018

Conversation

cdcapano
Copy link
Collaborator

This moves the stuff about sampling transforms out of BaseModel to its own class.

Depends on #35.

@cdcapano cdcapano added the on hold PR should not be merged yet because it depends on another PR or issue to be resolved first. label Jul 13, 2018
@cdcapano cdcapano requested a review from vivienr July 13, 2018 15:59
@pep8speaks
Copy link

pep8speaks commented Jul 13, 2018 •
edited
Loading

Hello @cdcapano! Thanks for updating the PR.

Cheers ! There are no PEP8 issues in this Pull Request. 🍻

Comment last updated on July 13, 2018 at 20:29 Hours UTC

@cdcapano cdcapano removed the on hold PR should not be merged yet because it depends on another PR or issue to be resolved first. label Jul 13, 2018
@coveralls
Copy link

Pull Request Test Coverage Report for Build 145

  • 14 of 42 (33.33%) changed or added relevant lines in 1 file are covered.
  • 1 unchanged line in 1 file lost coverage.
  • Overall coverage decreased (-0.05%) to 39.382%
Changes Missing Coverage Covered Lines Changed/Added Lines %
gwin/models/base.py 14 42 33.33%
Files with Coverage Reduction New Missed Lines %
gwin/models/base.py 1 56.11%
Totals Coverage Status
Change from base Build 141: -0.05%
Covered Lines: 1007
Relevant Lines: 2557
💛 - Coveralls

@cdcapano
Copy link
Collaborator Author

This is ready for merging now.

vivienr
vivienr approved these changes Jul 14, 2018
View reviewed changes
Copy link
Contributor

@vivienr vivienr left a comment

Choose a reason for hiding this comment

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

LGTM

@cdcapano cdcapano merged commit 581d836 into gwastro:master Jul 14, 2018
@cdcapano cdcapano deleted the sampling_transforms_class branch July 14, 2018 17:28
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@vivienr vivienr vivienr approved these changes

@cmbiwer cmbiwer Awaiting requested review from cmbiwer

@duncanmmacleod duncanmmacleod Awaiting requested review from duncanmmacleod duncanmmacleod is a code owner

Assignees

@vivienr vivienr

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@cdcapano @pep8speaks @coveralls @vivienr