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

[WIP] update scipy version #794

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

JosephMontoya-TRI
Copy link
Collaborator

Changes proposed in this pull request:

  • [WIP] for updating scipy version to inspect test differences.

@ardunn
Copy link
Collaborator

ardunn commented May 17, 2023

@JosephMontoya-TRI so according to this: https://docs.scipy.org/doc/scipy/reference/generated/scipy.interpolate.interp1d.html

I think the newest scipy changes what slinear does, though I'm not exactly sure how it differs from linear.

Just changing the interpolation keyword to be "linear" removes most of the errors for 'duplicate x', and the remaining seem to be problems with the precision of the interpolation like this:

for col_name in y_at_point.columns:
>           self.assertAlmostEqual(
                pred[col_name].iloc[0], y_at_point[col_name].iloc[0],
                places=2
            )
E           AssertionError: 3320.0375418686945 != 3319.9971375 within 2 places (0.040404368694453296 difference)

Edit: scipy linear does not do what I thought it did. Ignore above

@JosephMontoya-TRI
Copy link
Collaborator Author

Yeah I came to a similar conclusion the last time I tried this (which was like 3 years ago). I think we should probably just update the tests, change the interpolation keyword, and cross our fingers that this isn't breaking too many things downstream (if it's too problematic we can always change it back). @shijingsun-TRI @pkherring does that sound okay?

@pkherring
Copy link
Contributor

Yeah, I agree. Just update the test values and upgrade scipy

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.

3 participants