-
-
Notifications
You must be signed in to change notification settings - Fork 31
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
Use adaptive_grid on endpoints #68
Use adaptive_grid on endpoints #68
Conversation
I think the main issue is that you'll often want to pass |
that is already handled by the algorithm somehow, at least it doesn't error for me. It seems when plotting the NaNs are just skipped and the first interior point is used then. However, adjusting the |
I could fix #48 in this PR as well, if wanted. |
That'd be great, just remember that it will be breaking on Plots and other packages that use |
should we also add a So I will prepare a PR for Plots as well. |
Yes please :-) |
I don't think it does quite yet. I think you want to be able to refine the first and last intervals as well: PlotUtils.jl/src/adapted_grid.jl Lines 54 to 57 in 381fe0e
|
@mkborregaard This is ready for review. You need also my Plots-branch from JuliaPlots/Plots.jl#2204 to use the Plots interface. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't have anything to add - impressed by the test suite, which seems to cover all the right corners.
I have ported the test-functions mentioned in JuliaPlots/Plots.jl#621 (comment) and as far as I can see there is no visual difference between including the endpoints or not. But my pitchfork example shows that plotting different functions which start or end at the same point are definitely better off by including the endpoints.
This addresses #63, though the concerns of respecting log-scale and y-limits are still valid.
Failures on nightly seem unrelated to me.