| FazBrowse GitHub Viewer | Trending | | Home |
| Tools: [Download Repo ZIP] [Original HTTPS Page] |
Sorry, something went wrong.
There was a problem hiding this comment.
Thank you for opening your first PR into Matplotlib!
If you have not heard from us in a while, please feel free to ping @matplotlib/developers or anyone who has commented on the PR. Most of our reviewers are volunteers and sometimes things fall through the cracks.
You can also join us on gitter for real-time discussion.
For details on testing, writing docs, and our review process, please see the developer guide
We strive to be a welcoming and open project. Please follow our Code of Conduct.
Sorry, something went wrong.
|
I guess a question here is do we want to a) only allow a subset we think we know we will want (at this point the aria-label) b) only validate the type of the ones we know about and be permissive on any other keys c) validate and only allow known aria keys? c) is the most right, a) is probably too restrictive (we do not want to cut off someone who has a good idea) but exposes the least new API and b) is the least work, but leaves a bunch of foot cannons on the field. |
Sorry, something went wrong.
|
so i think there is future potential in using these aria features on lower level plot objects when rendering as svg. there is more detail that can be added through aria at that level of detail, that is a distant future, but a cool goal. since the artist._aria dict maps to html attributes the type is always dict[str, str]. i don't think there is any reason to restrict aria names because y'all haven't found specific things to use them for and, at the end of the day. we know folks are gonna do some bonkers things on the web. the extent of the checking we'd have to do is that the values are something that can be cast to an html string. i'm feeling a nice to have would be an alias specifically for aria label, to maybe encourage folks to use this feature in their blogs/docs. |
Sorry, something went wrong.
|
hey y'all. i'm working though the the pr checklist. where are good places to add docs for this feature? |
Sorry, something went wrong.
There was a problem hiding this comment.
Sorry, something went wrong.
the primary purpose of this is to encode web accessibility information (WAI-ARIA) into the artist. there is a new addition to ipython that includes alt text for images, this pull request is meant complement that so we have a place to pull the alt tags from. with the aria attribute we can include alt tech in a lot of documentation images.
these features will primarily be used with ipython, sphinx gallery, sphinx gallery to provide at aria-label/alt text for images.
i'd be happy with this solution too if it is easiest. for onboarding new folks to project, i'd really like arist.set_aria_label available though. the aria label is the only part we REALLY need from this. this way we could run sprints that have folks adding plt.set_alt_test on documentation for other projects. the aria system is something that web developers do in fact do weird things with, not sure we can stop that. |
Sorry, something went wrong.
|
I'll repeat the request for examples and a roadmap for how this will be used. |
Sorry, something went wrong.
|
This PR is affected by a re-writing of our history to remove a large number of accidentally committed files see discourse for details. To recover this PR it will need be rebased onto the new default branch (main). There are several ways to accomplish this, but we recommend (assuming that you call the matplotlib/matplotlib remote "upstream" git remote update
git checkout main
git merge --ff-only upstream/main
git checkout YOUR_BRANCH
git rebase --onto=main upstream/old_master
# git rebase -i main # if you prefer
git push --force-with-lease # assuming you are tracking your branchIf you do not feel comfortable doing this or need any help please reach out to any of the Matplotlib developers. We can either help you with the process or do it for you. Thank you for your contributions to Matplotlib and sorry for the inconvenience. |
Sorry, something went wrong.
|
At the very least this needs a rebase. Marking as stale for lack of response, and putting in draft. Feel free to move back to active... |
Sorry, something went wrong.
|
I can offer a use case. Some projects use notebooks containing Figures as a component of their sphinx docs, using e.g. nbsphinx to convert a notebook into something sphinx can eventually render as HTML. RdTools is one example: source notebook and built html. What I'd like to be able to do is specify figure-specific alt text in notebook code cells with something like fig.set_aria({'alt': 'Graph showing a time series of ...'}) (or maybe even plt.plot(..., alt='...')?) and have it eventually show up as <img alt="Graph showing a time series of ..." ...> in the sphinx output HTML. I think one way to get this working would be to store aria metadata on the Artist as this PR does, modify the matplotlib_inline backend to grab that metadata from the Figure and pass it along to the IPython display() call so that it ends up in the notebook cell output metadata, and modify nbsphinx to pass it along to sphinx (see spatialaudio/nbsphinx#646). Is this PR's approach considered workable? I'd be happy to submit the same patch in a fresh PR if so. |
Sorry, something went wrong.
|
@kanderso-nrel Feel free to take the actual commit, 2d28564 and start a new PR, keeping @tonyfast as an author (so ideally cherry-pick that commit and start your new PR from there). I believe that there are more things to be added (I haven't followed the discussion, but at least some roadmap ahead seems to be requested, as currently this allows users to add ARIA information, but it is not used anywhere). |
Sorry, something went wrong.
|
As discussed on the call this week, we need some more expert feedback on how to make sure this is useful. Things to consider:
This is not actually a blocker for #24309 as the .. plot directive already supports passing the alt text through from the rst and we need to work with sphinx-gallery to do the same in that context. |
Sorry, something went wrong.
|
Pushed to 3.8 as we are not going to get the right people together to sort this out in the next 2 weeks. |
Sorry, something went wrong.
Co-authored-by: Tim Hoffmann <2836374+timhoffm@users.noreply.github.com>
Co-authored-by: Kevin Anderson <kevin.anderson@nrel.gov>
| """ | ||
| Set ARIA properties to the artist. | ||
|
|
||
| A primary use of this method is to attach aria-label to the artist to |
There was a problem hiding this comment.
make this clearer that aria-description for raster backends (for alt text).
Sorry, something went wrong.
| if aria is not None: | ||
| raise TypeError( | ||
| f'aria must be dict or None, not {type(aria)}') | ||
| self._aria = aria |
There was a problem hiding this comment.
add validation to known aria attributes
Sorry, something went wrong.
| Back | FazBrowse Home | New Git URL |
PR Summary
This pull request addresses #15971 and adds WAI-ARIA Conventions to the artist class. This change provides a place for figures to include accessibility annotations.
This pull request is inspired by ipython/ipython#12864 which brings alt text to images in the IPython display. With this addition to matplotlib we'll be able improve author's abilities to include alt text with their figures.
PR Checklist