add default legend_color to eliminate KeyError when plotting arrow2d or disk #36460
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
DaveWitteMorris
changed the title
kwankyu
reviewed
Oct 14, 2023
|
|
||
| Verify that :trac:`36153` is fixed:: | ||
|
|
||
| sage: A = arrow2d((-1,-1), (2,3), legend_label="test") | ||
| """ |
|
Otherwise lgtm. |
|
Thanks, I didn't know that. I made the correction, so I think the PR is ready for review again. |
|
Thanks! |
vbraun
pushed a commit
to vbraun/sage
that referenced
this pull request
Oct 15, 2023
… plotting arrow2d or disk Fixes sagemath#36153. The line `g._legend_colors = [options['legend_color']]` appears in the code for seven graphics objects. Five of them (`circle`, `ellipse`, `line2d`, `point2d`, and `polygon2d`) set `None` as the default value for `legend_color`. The other two (`arrow2d` and `disk`) do not set a default, so a `KeyError` is raised if a `legend_label` is supplied, but no `legend_color` is specified. This bug was pointed out in sagemath#36153. This PR adds `None` as the default value of `legend_color` in `arrow2d` and `disk`. It also adds the corresponding doctest to all seven of these graphics objects, except `line2d`, which already has the example `line([(0,0),(1,1)], legend_label='line')`. The PR also corrects a typo in the docstring of the `cone` graphic object. ### 📝 Checklist - [x] The title is concise, informative, and self-explanatory. - [x] The description explains in detail what this PR is about. - [x] I have linked a relevant issue or discussion. - [x] I have created tests covering the changes. URL: sagemath#36460 Reported by: DaveWitteMorris Reviewer(s): Kwankyu Lee
vbraun
pushed a commit
to vbraun/sage
that referenced
this pull request
Oct 17, 2023
… plotting arrow2d or disk
Fixes sagemath#36153.
The line `g._legend_colors = [options['legend_color']]` appears in the
code for seven graphics objects. Five of them (`circle`, `ellipse`,
`line2d`, `point2d`, and `polygon2d`) set `None` as the default value
for `legend_color`. The other two (`arrow2d` and `disk`) do not set a
default, so a `KeyError` is raised if a `legend_label` is supplied, but
no `legend_color` is specified. This bug was pointed out in sagemath#36153.
This PR adds `None` as the default value of `legend_color` in `arrow2d`
and `disk`.
It also adds the corresponding doctest to all seven of these graphics
objects, except `line2d`, which already has the example
`line([(0,0),(1,1)], legend_label='line')`.
The PR also corrects a typo in the docstring of the `cone` graphic
object.
### 📝 Checklist
- [x] The title is concise, informative, and self-explanatory.
- [x] The description explains in detail what this PR is about.
- [x] I have linked a relevant issue or discussion.
- [x] I have created tests covering the changes.
URL: sagemath#36460
Reported by: DaveWitteMorris
Reviewer(s): Kwankyu Lee
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes #36153.
The line
g._legend_colors = [options['legend_color']]appears in the code for seven graphics objects. Five of them (circle,ellipse,line2d,point2d, andpolygon2d) setNoneas the default value forlegend_color. The other two (arrow2danddisk) do not set a default, so aKeyErroris raised if alegend_labelis supplied, but nolegend_coloris specified. This bug was pointed out in #36153.This PR adds
Noneas the default value oflegend_colorinarrow2danddisk.It also adds the corresponding doctest to all seven of these graphics objects, except
line2d, which already has the exampleline([(0,0),(1,1)], legend_label='line').The PR also corrects a typo in the docstring of the
conegraphic object.📝 Checklist