Skip to content

[MRG] Add alt text to scikit-learn documentation#21354

Open
isabela-pf wants to merge 10 commits intoscikit-learn:mainfrom
isabela-pf:altsprint
Open

[MRG] Add alt text to scikit-learn documentation#21354
isabela-pf wants to merge 10 commits intoscikit-learn:mainfrom
isabela-pf:altsprint

Conversation

@isabela-pf
Copy link
Copy Markdown

@isabela-pf isabela-pf commented Oct 16, 2021

Reference Issues/PRs

Addresses #21214. (I'm not using fixes because this does not cover the entire issue and I don't think it should be closed.)

What does this implement/fix? Explain your changes.

This PR adds alt text to some of scikit-learn's documentation. It includes alt text for

Any other comments?

This alt text was contributed during an alt text writing event by some wonderful volunteers, so this PR does not intend to cover adding alt text to all of scikit-learn's documentation.

As far as I'm aware, alt text is a new kind of contribution to this project. I'm happy to help discuss what this might include in terms of maintenance and/or review in the future if that is helpful.

Thanks in advance for reviewing this PR!

isabela-pf and others added 8 commits September 28, 2021 16:01
@isabela-pf
DOC add empty alt attributes
3fb300e
@isabela-pf @reshamas
Update doc/modules/biclustering.rst
20b8de6 
Co-authored-by: Reshama Shaikh <2507232+reshamas@users.noreply.github.com>
@isabela-pf @reshamas
DOC Add alt text to biclustering.rst
36c439f 
Co-authored-by: Reshama Shaikh <2507232+reshamas@users.noreply.github.com>
@isabela-pf
Merge branch 'altsprint' of https://github.com/isabela-pf/scikit-learn …
9387fb8 
…into altsprint
@isabela-pf @Madelene
@bsipocz @sunsyray @aguspesce @MarsBarLee @aerikpawson
DOC Add alt text to documentation
73f9fe3 
Co-authored-by: Madelene Campos <Madelene@users.noreply.github.com>
Co-authored-by: Brigitta Sipőcz <b.sipocz@gmail.com>
Co-authored-by: InessaPawson <albuscode@gmail.com>
Co-authored-by: sunsyray <33903999+sunsyray@users.noreply.github.com>
Co-authored-by: Agustina <pesce.agustina@gmail.com>
Co-authored-by: MarsBarLee <46167686+MarsBarLee@users.noreply.github.com>
Co-authored-by: Aerik Pawson <45904740+aerikpawson@users.noreply.github.com>
@isabela-pf
DOC Add alt text to covariance.rst
a188cfa
@isabela-pf
DOC Remove unused alt tags
b4903aa
@isabela-pf
DOC Edit alt text formatting
90952b5
@isabela-pf isabela-pf mentioned this pull request Oct 16, 2021
Closed
33 tasks
@ogrisel
Copy link
Copy Markdown
Member

ogrisel commented Oct 18, 2021 •
edited
Loading

Thanks for the PR. Apparently there is a problem with the rendering of the figure directives with multi-line alt texts, see the "Details" link in the Continuous Integration status report:

image

Clicking this link will lead you to a page with links to HTML rendered pages of the documentation for the edited source and a summary of the sphinx problems:

Sphinx Warnings in affected files

    doc/modules/biclustering.rst:47: WARNING: Error in "figure" directive:
    doc/modules/biclustering.rst:61: WARNING: Error in "figure" directive:
    doc/modules/calibration.rst:53: WARNING: Error in "figure" directive:
    doc/modules/clustering.rst:466: WARNING: Error in "image" directive:
    doc/modules/clustering.rst:466: WARNING: Substitution definition "noisy_img" empty or invalid.
    doc/modules/clustering.rst:472: WARNING: Error in "image" directive:
    doc/modules/clustering.rst:472: WARNING: Substitution definition "segmented_img" empty or invalid.
    doc/modules/clustering.rst:818: WARNING: Error in "image" directive:
    doc/modules/clustering.rst:818: WARNING: Substitution definition "dbscan_results" empty or invalid.
    doc/modules/clustering.rst:480: WARNING: Undefined substitution referenced: "noisy_img".
    doc/modules/clustering.rst:480: WARNING: Undefined substitution referenced: "segmented_img".
    doc/modules/clustering.rst:826: WARNING: Undefined substitution referenced: "dbscan_results".
    doc/modules/covariance.rst:153: WARNING: Error in "figure" directive:
    doc/modules/covariance.rst:180: WARNING: Error in "figure" directive:
    doc/modules/covariance.rst:218: WARNING: Error in "figure" directive:
    doc/modules/covariance.rst:346: WARNING: Error in "image" directive:
    doc/modules/covariance.rst:346: WARNING: Substitution definition "robust_vs_emp" empty or invalid.
    doc/modules/covariance.rst:368: WARNING: Undefined substitution referenced: "robust_vs_emp".

I suspect that the indentation level of the subsequent lines is incorrect.

@ogrisel
Copy link
Copy Markdown
Member

ogrisel commented Oct 18, 2021

To fix those, I would advise you to try to build the documentation locally instead of relying on the slow CI of the pull request. Here are instructions to render the documentation locally:

https://scikit-learn.org/stable/developers/contributing.html#documentation

Since you want some of the figures of the documentation to be generated to check the alt text of the generated <img /> tags, it's a good idea to set the EXAMPLE_PATTERN environment variable when building the doc locally. For instance, when working on the biclustering section of the user guide, you can build the doc with:

EXAMPLES_PATTERN=".*biclustering.*" make html

The .* characters are regexp wildcards and therefore the examples/bicluster/plot_spectral_coclustering.py filename should be matched the by above pattern and the example file should be executed to render the .png files for the biclustering section of the doc.

ogrisel
ogrisel reviewed Oct 18, 2021
View reviewed changes
Copy link
Copy Markdown
Member

@ogrisel ogrisel left a comment •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here are a first few comments. I will do a proper review once you find a way to fix the sphinx build with the help of the above comment.

doc/modules/biclustering.rst Outdated
:align: center
:scale: 50
:alt: The graph is a square heat map, 5x5, with axes from 0 to 250. The darkest 5
squares of heat map run diagonally from top left to bottom right.
Copy link
Copy Markdown
Member

@ogrisel ogrisel Oct 18, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The plot is a 250x250 heatmap with an almost block-diagonal structure with 5 darker shaded rectangular blocks of varying shapes on the diagonal and a lighter shaded values outside. Within each blocks the values are almost uniform but not exactly with small random fluctuations.

Copy link
Copy Markdown
Author

@isabela-pf isabela-pf Nov 28, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since it hasn't been said on this PR yet, I'm not someone that uses scikit-learn or even knows most of the figures we wrote alt text for at this sprint, so I'm happy to defer to your expertise on how much information is helpful to properly describe the figure in context.

As someone who does know alt text, though, I know two of the things I check alt text for is that is 1. that it describes image in a way that makes sense in the context it's used in 2. that it is a short as possible. I don't think this edit adds much to the meaning of the figure in the documentation since it seems to add details rather than the main features of the figure. Of course, if I'm misunderstanding please let me know and I'll commit this suggestion.

A good explanation of what I'm talking about can be found on WebAIM's discussion of alt text written for proper context.

isabela-pf and others added 2 commits November 3, 2021 14:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@ogrisel ogrisel ogrisel left review comments

At least 1 approving review is required to merge this pull request.

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@isabela-pf @ogrisel