More uniform independent vars

[newville]

Description

This is a change to Model to make the assignment of independent variables from function arguments a bit clearer. This is a more complete version of what @schtandard was trying to do with #925 and #926 (and apologies for taking so long on this!)

With this change, function arguments with non-numeric default values will now be listed as independent variables, by default. That is, with a mode function of

def mymodel(x, center=5, check_positive=True):
  . ...

mod = Model(mymodel)
print(mod.independent_vars)

will now give ['x', 'check_positive']. The default value will be kept and used as a default value.

There is sort of an interesting case of lmfit.lineshapes.voigt, which has

def voigt(x, amplitude=1.0, center=0.0, sigma=1.0, gamma=None):
    """Return a 1-dimensional Voigt function.
        ...
    """
    if gamma is None:
        gamma = sigma
    ...

With the change here, gamma is initially listed as an independent variable, with default value None. But it can still be turned into a Parameter to vary (or, I suppose constrain some other way) in the fit. I sort of struggled with this, and currently this "turn a function argument that would be an independent variable into a Parameter" can only be done when the default value is None or a boolean (True / False)

After review, this should probably be squashed and merged.

Type of Changes

• Bug fix
• New feature
• Refactoring / maintenance
• Documentation / examples

Tested on

Python: 3.11.5 | packaged by conda-forge | (main, Aug 27 2023, 03:35:23) [Clang 15.0.7 ]

lmfit: 1.2.2.post35+gc9c2b2d.d20240223, scipy: 1.11.4, numpy: 1.26.2, asteval: 0.9.31, uncertainties: 3.1.7

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?

[codecov]

Codecov Report

Attention: Patch coverage is 95.23810% with 3 lines in your changes are missing coverage. Please review.

Project coverage is 93.14%. Comparing base (c9c2b2d) to head (943b976).
Report is 4 commits behind head on master.

Files	Patch %	Lines
lmfit/model.py	96.29%	2 Missing ⚠️
lmfit/models.py	88.88%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master     #941      +/-   ##
==========================================
+ Coverage   93.11%   93.14%   +0.02%     
==========================================
  Files          10       10              
  Lines        3720     3762      +42     
==========================================
+ Hits         3464     3504      +40     
- Misses        256      258       +2     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[reneeotten]

@newville I will take a look at this during the weekend

[newville]

@reneeotten Thanks - that's much appreciated.

This is a bit "deep in the weeds" and mostly implementation detail. I think which function arguments to a Model Function should become "independent variable" can be a bit subtle. But the "Model.opts" in the current implementation is confusing, and @schtandard raised some good points. I am not sure there is one obvious solution, but I think this is an improvement.

[reneeotten]

sorry @newville for the delay.... This looks good to me, I've actually only a few comments regarding some documentation typos and one question regarding an actual code change. Other than that I am happy for it to be merged.

I'll comment later in the discussion about making a new release. I think most is addressed; there was one question on the mailinglist a while ago that involved basinhopping I think that I was going to look into. I'll try to do so later today and then we can probably do some minor clean-up for a release (i.e., update whatsnew.rst, etcetera) and we should be good to go!

[reneeotten]

@newville also, unrelated to this PR, but: I guess we can remove the ampgo repository in the lmfit organization. That code has been included already so we don't need to keep the fork.

[newville]

Thanks very much I'll fix the typos and bad wording, and answer the typing question.

I'm okay with removing ampgo or leaving it for historical purposes. No preference.
As you no doubt saw, we had uncertainties moved to the lmfit organization and now there is a lot of development there -- I can't keep up!
And, using the same kind of reason that "code is better in GitHub organizations", I moved asteval here too. It just seems safer.

I don't remember the details of the basinhopping question, but I agree to look again and see if there is something that can be fixed. It does feel like it is time for a new release, but waiting a few weeks is not a problem!

[reneeotten]

thanks @newville - a squash-merge with updating the final commit message would be good here.

[newville]

@reneeotten fixed the commit typo, a docstring typo, waiting for tests to complete.

For basinhopping, I think this is working okay. I would like to add an example, similiar to example_diffev.py that uses basin-hoppping by default, but can easily be called to compare other methods.

I know it is unrelated to this PR, but I think adding an example is not a problem, so I just added that example here.

[newville]

@reneeotten I'll plan to squash and merge this in a few days.