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

Update ConfidenceInterval class constructor parameters #937

Merged
merged 5 commits into from
Jan 26, 2024

Conversation

Anselmoo
Copy link
Contributor

@Anselmoo Anselmoo commented Jan 23, 2024

Defining self.min_rel_change in confidence.py as variable Fixes #936

Description

This pull request updates the constructor parameters of the ConfidenceInterval class in confidence.py. Specifically, it adds a new parameter called min_rel_change with a default value of 1e-5. This change addresses issue #936.

Type of Changes
  • Bug fix
  • New feature
  • Refactoring / maintenance
  • Documentation / examples
Tested on
Verification

Have you

  • included docstrings that follow PEP 257?
  • referenced existing Issue and/or provided relevant link to mailing list?
  • verified that existing tests pass locally?
  • verified that the documentation builds locally?
  • squashed/minimized your commits and written descriptive commit messages?
  • added or updated existing tests to cover the changes?
  • updated the documentation and/or added an entry to the release notes (doc/whatsnew.rst)?
  • added an example?

Defining `self.min_rel_change` in confidence.py as variable
Fixes lmfit#936
@Tillsten
Copy link
Contributor

I fully agree with addition. I think a test showing that changing the parameter does something useful would be nice.

@newville
Copy link
Member

@Anselmoo Thanks -- I agree with this being better as a command-line argument. So, Yes, Thanks!

I'm not sure I understand all the test failures, but bear with us as we try to figure out what is causing them.

lmfit/confidence.py Outdated Show resolved Hide resolved
…l class

According to review, set to the end of the functions

Fix formatting in conf_interval function
Copy link
Contributor

@reneeotten reneeotten left a comment

Choose a reason for hiding this comment

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

thanks @Anselmoo ; it looks like that correcting the position of the new argument fixed the tests as well. This looks good to me, except for a few changes that are required in the docstring.

lmfit/confidence.py Outdated Show resolved Hide resolved
lmfit/confidence.py Outdated Show resolved Hide resolved
lmfit/confidence.py Outdated Show resolved Hide resolved
lmfit/confidence.py Show resolved Hide resolved
@newville
Copy link
Member

@Anselmoo Thanks - that looks good to me.

@reneeotten reneeotten merged commit 6096c3a into lmfit:master Jan 26, 2024
14 checks passed
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.

Defining self.min_rel_change in confidence.py as variable
4 participants