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

Rename the likelihood module to models #35

Merged
merged 29 commits into from
Jul 13, 2018

Conversation

cdcapano
Copy link
Collaborator

@cdcapano cdcapano commented Jul 6, 2018

This renames the likelihood module to models. Also renames all instances of likelihood_evaluator to model in the sampler module and elsewhere. The BaseLikelihoodEvaluator has been renamed Model and DataBasedLikelihoodEvaluator has been renamed DataModel.

Depends on #33.

@pep8speaks
Copy link

pep8speaks commented Jul 6, 2018

Hello @cdcapano! Thanks for updating the PR.

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

Comment last updated on July 12, 2018 at 08:28 Hours UTC

@coveralls
Copy link

coveralls commented Jul 6, 2018

Pull Request Test Coverage Report for Build 134

  • 103 of 191 (53.93%) changed or added relevant lines in 12 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage decreased (-0.04%) to 39.437%

Changes Missing Coverage Covered Lines Changed/Added Lines %
gwin/workflow.py 0 1 0.0%
gwin/models/init.py 4 5 80.0%
gwin/sampler/mcmc.py 7 8 87.5%
gwin/sampler/kombine.py 10 13 76.92%
gwin/io/hdf.py 5 12 41.67%
gwin/sampler/emcee.py 19 29 65.52%
gwin/option_utils.py 5 16 31.25%
gwin/models/analytic.py 9 21 42.86%
gwin/sampler/base.py 11 27 40.74%
gwin/models/base.py 23 49 46.94%
Totals Coverage Status
Change from base Build 130: -0.04%
Covered Lines: 1008
Relevant Lines: 2556

💛 - Coveralls

duncanmmacleod
duncanmmacleod previously approved these changes Jul 6, 2018
Copy link
Member

@duncanmmacleod duncanmmacleod 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
Copy link
Collaborator Author

Also renamed sampling_parameters --> sampling_params, variable_args --> variable_params; static_args --> static_params to make terminology consistent.

This depends on the next pycbc release, so putting on hold until that's out (should be today or tomorrow).

@cdcapano cdcapano added on hold PR should not be merged yet because it depends on another PR or issue to be resolved first. and removed work in progress labels Jul 12, 2018
@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 12, 2018
@cdcapano
Copy link
Collaborator Author

Actually, looks like I can merge this now without failing Travis. This will break gwin if you don't have current pycbc master installed, but the new release should come out in the next few days. Would be good to merge this now, to simplify #36.

@vivienr
Copy link
Contributor

vivienr commented Jul 13, 2018

Should we have codeclimate setting's relaxed a little?

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 7454fad into gwastro:master Jul 13, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants