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

Rewrite #224

Closed
3 of 7 tasks
timholy opened this issue Aug 11, 2018 · 9 comments
Closed
3 of 7 tasks

Rewrite #224

timholy opened this issue Aug 11, 2018 · 9 comments

Comments

@timholy
Copy link
Member

timholy commented Aug 11, 2018

This is to give heads-up about some big coming changes. I'll check them off as I complete them locally and put it all in one gigantic PR.

In addition to the agenda in #212,

  • drop support for 0.6 and get rid of Compat
  • stop exporting gradient, gradient!, gradient1, hessian, hessian!, and hessian1.
  • deprecate itp[x] in favor of itp(x)
  • (not done, but underway) get rid of all the generated functions
  • don't use clamp for interpolation objects (make them throw BoundsErrors)
  • consider renaming OnGrid to OnKnot
  • improve performance
@tomasaschan
Copy link
Contributor

tomasaschan commented Aug 13, 2018

stop exporting gradient, gradient!, gradient1, hessian, hessian!, and hessian1.

👍 🎉

drop support for 0.6 and get rid of Compat

As noted elsewhere: I think it makes sense to keep the window where we support 0.6 and 0.7/1.0 open for as long as possible, to give people a chance to update their code. (Deprecation warnings for stuff we need to remove when dropping 0.6 is fine, of course.)

(not done, but underway) get rid of all the generated functions

Nice!

improve performance

I wouldn't believe it was you coding if you didnt... 😄

@tomasaschan
Copy link
Contributor

I must be really unattentive today. Read this, and yet went ahead and filed #225 anyway. Feel free to close that and move on with your local version instead, if the git-juggling feels unnecessarily complex.

Otherwise, I think #225 might be the quickest way to 1.0-compatibility (at least as far as our test suite knows 😅).

@timholy
Copy link
Member Author

timholy commented Aug 13, 2018

I completely disagree with supporting 0.6 at the same time as 0.7. By bumping the minimum julia version in REQUIRE we allow people still on 0.6 to run "old" versions of Interpolations that continues to work on 0.6. Only people running 0.7 and higher will get the new code.

0.7/1.0 is the future, and it's also here now. So if we're doing a big update there is no reason to cruft the code.

What I'm striving for is to do all our breaking at once. It seems we also want to deprecate the itp[x] syntax. Right now people are in an upgrade cycle and so will view depwarns from Interpolations as part of background noise; a month from now they might be annoyed ("I thought I was done fixing depwarns!")

@tomasaschan
Copy link
Contributor

That are very good points; I didn't consider the option of pinning older Interpolations versions if you're stuck on 0.6 for some reason.

@sglyon
Copy link
Member

sglyon commented Aug 13, 2018

I think the pinning would happen automatically as the julia 0.6 package manager would get the newest Interpolations that supports 0.6.

@timholy
Copy link
Member Author

timholy commented Aug 13, 2018

What do folks think about renaming OnGrid to OnKnot? I can never remember what OnCell and OnGrid mean. (See the edited TODO list above.)

Maybe even consider changing them to AtCellCenter and AtKnot?

@tomasaschan
Copy link
Contributor

I agree that they are confusing. I think the main reasons are that

a) it's a little unclear what a grid and a cell are, respectively, in the context of interpolation. Most users just think of interpolation as "finding values between my data points, with varying smoothness of the curve" - you have to start caring about how interpolations are mathematically defined to understand what a cell and a grid are (and that goes for a knot too, btw).

b) it's a little unclear what we are placing on cells/grids/knots. From context you can assume that "it's (probably) the data", but there's nothing in the name to suggest that. Maybe this is OK.

I don't know what better names would be - I'm not sure I'm less confused by AtCellCenter and AtKnot than I am by OnGrid and OnCell - but if better names are made up I'll be the first to applaud it. (And if someone else is less confused, they're better even if I still am...)

@tknopp
Copy link
Member

tknopp commented Sep 8, 2018

@tlycken: Just want support @timholy that most packages have dropped 0.6 support on master and let 0.6 and 0.7/1.0 diverge. And for 0.6 users this is the safest thing you can get, because a 0.6/0.7 code base has always the risk that 0.7 changes break the 0.6 code. This has happened in the past (e.g. axis was introduced into Compat 0.6 and broke AxisArrays)

@timholy
Copy link
Member Author

timholy commented Sep 18, 2018

Closed by #226

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

No branches or pull requests

4 participants