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
Allow custom fill endpoints in a filled plot #14599
Comments
Author: Sergey Bykov |
Commit: |
New commits:
|
comment:7
This is great that you are working on this! I don't see any obvious problems in the code, though I won't be able to look at it more carefully right now. My first thought, though, is what happens with parametric plots? Somehow their fill "magically" works right, and I'm wondering what the behavior might be with this keyword. |
Branch pushed to git repo; I updated commit sha1. New commits:
|
comment:9
Replying to @kcrisman:
Thanks for the feedback. You were right, there was some weird staff with parametric curves. It seems to work fine now, but these new custom endpoints definitely require some additional checks. However, I couldn't break it with my test suite, which contains a set of well-known curves. |
comment:10
Hmm, maybe you can add some of those to the tests/examples; at any rate, currently your branch has only one non-example, not examples of how to use this. Or, if you have a pretty extensive suite, there are a couple testing-only files out there that could be added to - we love tests! |
Branch pushed to git repo; I updated commit sha1. New commits:
|
comment:12
Look at the following interesting behavior.
It's definitely obeying the Similarly, I'm not sure that
is behaving as expected. And I'm completely flummoxed as to why
due to the new display hook stuff! One possible workaround would be to disallow the custom fill (for now) in parametric plots (i.e. raise an error) and make that a new ticket. I do think that the first 'bad' example should be dealt with, though. |
comment:13
the problem is probably that the plot object is invalid (it can't actually show a plot). Because there is insufficient validation this only causes issues later when you actually try to plot it. |
comment:14
Hmm, that makes sense. I have to say that the error is a little cryptic. |
comment:15
Removing the "beginner" tag from old tickets. Some could be returned to beginner-friendly status by adding a comment about what needs to be done. Some others might be easy for an experienced developer to finish. |
In this ask.sagemath question, the issue is raised whether
should be doable in a single command, such as with a
fillxmin
/fillxmax
keyword (or some better name).CC: @sagetrac-dsm @vbraun
Component: graphics
Author: Sergey Bykov
Branch/Commit: u/captaintrunky/allow_custom_fill_endpoints_in_a_filled_plot @
2f0fdcb
Issue created by migration from https://trac.sagemath.org/ticket/14599
The text was updated successfully, but these errors were encountered: