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

Fixed a bug in ompy.library.log_interp1d #199

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

vetlewi
Copy link
Collaborator

@vetlewi vetlewi commented Aug 8, 2022

I've fixed a minor bug in ompy.library.log_interp1d which prevented the user to set the kind keyword arguments to scipy.interpolate.interp1d. The default value is now set to kind='linear' and can be changed by the user.

I'm also considering adding a lin_interp1d to ompy.library. Mainly since it could complement the log_interp1d.

def lin_interp1d(xx, yy, **kwargs):
    """ Interpolate a 1-D function.logarithmically """
    kwargs.setdefault('kind', 'linear')
    lin_interp = interp1d(xx, yy, **kwargs)
    return lin_interp

Any objections?

One could also redo the two interpolation functions into a single function interp1d(xx, yy, type='log/lin', **kwargs) but that would require a little work to properly propagate the change to the entire package, and would not be backwards compatible if any users have used it explicitly. One could preserve the current function as a "legacy" solution with the 'new' as a wrapper:

def interp1d(xx, yy, type='log', **kwargs):
    if type == 'lin':
        return lin_interp1d(xx, yy, **kwargs)
    if type == 'log':
        return log_interp1d(xx, yy, **kwargs)
    else:
        raise TypeError(f"Unsupported 'type' keyword: '{type}'")

and add a depreciation warning when the legacy versions are used.

I've fixed a minor bug in log_interp1d which prevented the propper propagation of keyword arguments to the interp1d class from scipy. The 'kind' keyword will now be set as a default in the keyword arguments, thus can be controlled by the user.
@vetlewi vetlewi marked this pull request as draft January 18, 2023 18:12
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.

None yet

1 participant