Skip to content

DOC: add new showcase example, replace gendered one #23365

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 2 commits into from
Jun 30, 2022
Merged

DOC: add new showcase example, replace gendered one #23365

merged 2 commits into from
Jun 30, 2022

Conversation

jklymak
Copy link
Member

@jklymak jklymak commented Jun 29, 2022 •
edited
Loading

PR Summary

We really don't need to have examples that refer to gender like: https://matplotlib.org/stable/gallery/showcase/bachelors_degrees_by_gender.html. Replace it by this example.. I cleaned up some of the example, which was doing a lot of unnecessary manual futzing, but I left some of it in.

PR Checklist

Tests and Styling

  • Has pytest style unit tests (and pytest passes).
  • Is Flake 8 compliant (install flake8-docstrings and run flake8 --docstring-convention=all).

Documentation

  • New features are documented, with examples if plot related.
  • New features have an entry in doc/users/next_whats_new/ (follow instructions in README.rst there).
  • API changes documented in doc/api/next_api_changes/ (follow instructions in README.rst there).
  • Documentation is sphinx and numpydoc compliant (the docs should build without error).

@jklymak jklymak changed the title DOC: add new shwocase example, replace gendered one DOC: add new showcase example, replace gendered one Jun 29, 2022
@jklymak jklymak force-pushed the doc-rework-showcase-multiline branch 3 times, most recently from 3fa814f to a30c94c Compare June 29, 2022 11:59
@jklymak jklymak marked this pull request as ready for review June 29, 2022 12:22
@jklymak
Copy link
Member Author

jklymak commented Jun 29, 2022

Here is a rendering: https://output.circle-artifacts.com/output/job/72817ef5-b72b-4b17-adae-7f407b18e978/artifacts/0/doc/build/html/gallery/showcase/stock_prices.html

oscargus
oscargus reviewed Jun 29, 2022
View reviewed changes
examples/showcase/stock_prices.py Outdated
Stock prices over 32 years
==========================

.. redirect-from:: /gallery/showcase/backelor_degrees_by_gender
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
.. redirect-from:: /gallery/showcase/backelor_degrees_by_gender
.. redirect-from:: /gallery/showcase/bachelors_degrees_by_gender

oscargus
oscargus reviewed Jun 29, 2022
View reviewed changes
examples/showcase/stock_prices.py Outdated
'#e377c2', '#f7b6d2', '#7f7f7f', '#c7c7c7', '#bcbd22', '#dbdb8d',
'#17becf', '#9edae5'])

# 'AMZN', 'DELL', 'GOOGL', 'ADBE', '^GSPC', '^IXIC']
Copy link
Member

Choose a reason for hiding this comment

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

?

oscargus
oscargus reviewed Jun 29, 2022
View reviewed changes
examples/showcase/stock_prices.py Outdated
# is adding specific offsets y position because some labels overlapped.
y_pos = stock_data[column][-1]

# use an offset transform, in points, for any text that needs to be nudged
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
# use an offset transform, in points, for any text that needs to be nudged
# Use an offset transform, in points, for any text that needs to be nudged

oscargus
oscargus reviewed Jun 29, 2022
View reviewed changes
examples/showcase/stock_prices.py Outdated
stocks_ticker = ['IBM', 'AAPL', 'MSFT', 'XRX', 'AMZN', 'DELL', 'GOOGL',
'ADBE', 'GSPC', 'IXIC']

# manually adjust the label positions vertically (units are points = 1/72 inch)
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
# manually adjust the label positions vertically (units are points = 1/72 inch)
# Manually adjust the label positions vertically (units are points = 1/72 inch).

oscargus
oscargus approved these changes Jun 29, 2022
View reviewed changes
Copy link
Member

@oscargus oscargus 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.

Some minor nit-picks (but the redirect is relevant to change).

And a good idea to change this! (I think there are some other examples with a similar subject.)

@jklymak jklymak force-pushed the doc-rework-showcase-multiline branch from a30c94c to cac2bc6 Compare June 29, 2022 13:01
@jklymak
Copy link
Member Author

jklymak commented Jun 29, 2022

And a good idea to change this! (I think there are some other examples with a similar subject.)

Yes for sure. I thought those would be easy as a good first issue, but this one was a bit more involved:

#23352

@tacaswell
Copy link
Member

Can we please hold on merging this for a bit?

@story645 story645 added the status: needs comment/discussion needs consensus on next step label Jun 29, 2022
@jklymak jklymak marked this pull request as draft June 29, 2022 15:54
@jklymak
Copy link
Member Author

jklymak commented Jun 29, 2022

I moved to draft, but just to be clear

  1. the example this replaces can easily be construed to be problematic. I strongly feel the docs should be as neutral as possible with respect to anything societal.
  2. the example this replaces has no data attribution so, for all we know, it is completely made up random red noise. I strongly hope no one is actually using this data to form opinions about anything.

@tacaswell tacaswell marked this pull request as ready for review June 30, 2022 15:01
@tacaswell tacaswell added this to the v3.6.0 milestone Jun 30, 2022
@tacaswell
Copy link
Member

(sorry, accidentally posted a half-written message which I deleted)

I had not realized that we did not have a citation on the underlying data. Although I trust that this data come to us in good faith, the lack of provenance is enough to convince me that we should change the example. I do not think it is a good use of our time to track down (and evaluate) the source of the original data.

The DEI issues in STEM are obvious and far (far) larger than Matplotlib. We should focus on making sure we are an open welcoming project that treats everyone who comes to us with respect and professionalism to enable them to contribute as they have ability and desire. The contents of an example that is, in the best case, a distraction.

@tacaswell tacaswell merged commit d89157b into matplotlib:main Jun 30, 2022
@tacaswell tacaswell removed the status: needs comment/discussion needs consensus on next step label Jul 30, 2022
@tacaswell
Copy link
Member

There was (offline) discussion about this before it was merged, I should have made that clear.

I have since learned (via @noatamir and @melissawm ) the data is likely from https://www.npr.org/sections/money/2014/10/21/357629765/when-women-stopped-coding / https://www.nsf.gov/statistics/nsf13327/content.cfm?pub_id=4266&id=2 .

@story645 story645 added status: needs comment/discussion needs consensus on next step and removed status: needs comment/discussion needs consensus on next step labels Jul 31, 2022
@jklymak jklymak deleted the doc-rework-showcase-multiline branch August 11, 2022 19:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@oscargus oscargus oscargus approved these changes

Assignees
No one assigned
Labels
Documentation
Projects
None yet
Milestone
v3.6.0
Development

Successfully merging this pull request may close these issues.
4 participants
@jklymak @tacaswell @oscargus @story645