Skip to content

MAINT: remove blacklist/whitelist terms#16669

Merged
mattip merged 3 commits intonumpy:masterfrom
erfannariman:fix_language_style
Jun 24, 2020
Merged

MAINT: remove blacklist/whitelist terms#16669
mattip merged 3 commits intonumpy:masterfrom
erfannariman:fix_language_style

Conversation

@erfannariman
Copy link
Copy Markdown

@erfannariman erfannariman commented Jun 23, 2020

@erfannariman
Copy link
Copy Markdown
Author

erfannariman commented Jun 23, 2020

Hmm, not sure why travis is failing, anyone an idea?

@alimcmaster1
Copy link
Copy Markdown

alimcmaster1 commented Jun 23, 2020

Great! Could we just use ‘allowlist’ opposed to safelist?

@erfannariman
Copy link
Copy Markdown
Author

erfannariman commented Jun 23, 2020 •
edited
Loading

Great! Could we just use ‘allowlist’ opposed to safelist?

Yes, wanted to, but then I saw that travis themself changed it to safelist, see: https://github.com/travis-ci/apt-package-safelist

rossbar
rossbar reviewed Jun 23, 2020
View reviewed changes
Copy link
Copy Markdown
Contributor

@rossbar rossbar left a comment

Choose a reason for hiding this comment

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

Why the name changes in numpy/tests/test_public_api.py? This is unrelated to the Travis config.

Note also that https://github.com/travis-ci/apt-package-whitelist automatically redirects to the correct URL.

@erfannariman
Copy link
Copy Markdown
Author

erfannariman commented Jun 23, 2020

Why the name changes in numpy/tests/test_public_api.py? This is unrelated to the Travis config.

Note also that https://github.com/travis-ci/apt-package-whitelist automatically redirects to the correct URL.

Not sure if I understand you correctly. The purpose behind this PR is to remove words like blacklist or whitelist, not any purpose for travis if that is what you meant. To quote one of the sources I linked to:

The terms “allowlist” and “blocklist” describe
their purpose, while the other words use metaphors to
decribe their purpose.

@rossbar
Copy link
Copy Markdown
Contributor

rossbar commented Jun 23, 2020

I see - sorry, I was missing the context. I thought the main purpose was to correct a link in the comments in the CI config.

@erfannariman
Copy link
Copy Markdown
Author

erfannariman commented Jun 23, 2020

I see - sorry, I was missing the context. I thought the main purpose was to correct a link in the comments in the CI config.

No problem, thank you for taking the time to review.

@rossbar
Copy link
Copy Markdown
Contributor

rossbar commented Jun 23, 2020

A quick grep shows there are a couple other instances of "blacklist" as well. Most are in release notes/change logs and probably shouldn't be changed, but it might be worth getting the instances that are still in the source and bundling these all up in one PR.

@erfannariman
Copy link
Copy Markdown
Author

erfannariman commented Jun 24, 2020

Added the rest I could find, if I see correctly there are only release notes/logs left:

$ grep -rnw '.' -e 'blacklist'
./doc/changelog/1.12.0-changelog.rst:328:* `#7518 <https://github.com/numpy/numpy/pull/7518>`__: BUG: Extend glibc complex trig functions blacklist to glibc <...
./doc/changelog/1.12.0-changelog.rst:548:* `#8319 <https://github.com/numpy/numpy/pull/8319>`__: BLD: blacklist powl (longdouble power function) on OS X.
./doc/changelog/1.13.0-changelog.rst:150:* `#8318 <https://github.com/numpy/numpy/pull/8318>`__: BLD: blacklist powl (longdouble power function) on OS X.
./doc/changelog/1.13.2-changelog.rst:32:* `#9580 <https://github.com/numpy/numpy/pull/9580>`__: BUG: Add hypot and cabs functions to WIN32 blacklist.
./doc/changelog/1.13.3-changelog.rst:58:* `#9580 <https://github.com/numpy/numpy/pull/9580>`__: BUG: Add hypot and cabs functions to WIN32 blacklist.
./doc/source/release/1.11.1-notes.rst:14:- #7535 BUG: Extend glibc complex trig functions blacklist to glibc < 2.18.
./doc/source/release/1.13.2-notes.rst:45:* #9580 BUG: Add hypot and cabs functions to WIN32 blacklist.
./doc/source/release/1.13.3-notes.rst:49:* #9580 BUG: Add hypot and cabs functions to WIN32 blacklist.

@rossbar

rossbar
rossbar approved these changes Jun 24, 2020
View reviewed changes
Copy link
Copy Markdown
Contributor

@rossbar rossbar left a comment

Choose a reason for hiding this comment

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

LGTM - this catches all the instances of "blacklist" and "whitelist" given by grep -ri excepting those in the change logs and release notes. Thanks @erfannariman

@charris
Copy link
Copy Markdown
Member

charris commented Jun 24, 2020

I don't approve of this abuse of language.

@erfannariman
Copy link
Copy Markdown
Author

erfannariman commented Jun 24, 2020

I don't approve of this abuse of language.

Not sure what you mean, could you elaborate?

@rgommers
Copy link
Copy Markdown
Member

rgommers commented Jun 24, 2020

@charris you may be missing the bigger picture here (it isn't explained in the PR description, which would've been nice), this isn't just a random idea copied from Chrome. The point of this is to remove terms that are racially/historically charged, like blacklist/whitelist (and "master"). As you can see in this PR, TravisCI made this change. GitHub is changing its default branch name to "main": link.

It may seem like a small thing, but if these terms make people uncomfortable, it's also an easy thing to change.

And: it's definitely not worth arguing against, the discomfort Black people (and probably other minorities as well) experience is real. So let's merge this.

@rgommers rgommers changed the title Fixed language style MAINT: remove blacklist/whitelist terms Jun 24, 2020
@charris
Copy link
Copy Markdown
Member

charris commented Jun 24, 2020

It may seem like a small thing, but if these terms make people uncomfortable

Blacklist is an old English word with a well established meaning. I am aware of the larger picture and it makes me uncomfortable. "First they came for the Socialists...".

@rgommers
Copy link
Copy Markdown
Member

rgommers commented Jun 24, 2020

I am aware of the larger picture and it makes me uncomfortable.

The sentiment on this I've seen expressed a number of times: https://twitter.com/techgirl1908/status/1272687599732154368

"First they came for the Socialists...".

That is highly inappropriate Chuck, kind of upside down. I respectfully ask you don't make this into a big discussion. Everyone else on this PR is fine with it, as are all the Pandas devs in the PR that motivated this one. If you can't emphathize with the discomfort others have with these terms, please just don't comment.

eric-wieser
eric-wieser reviewed Jun 24, 2020
View reviewed changes
Comment thread numpy/core/src/common/npy_config.h
#include "numpy/npy_cpu.h"
#include "numpy/npy_os.h"

/* blacklist */
Copy link
Copy Markdown
Member

@eric-wieser eric-wieser Jun 24, 2020

Choose a reason for hiding this comment

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

If we're going to change this, I think we should expand on this comment. There are various issues around the issue tracker referring to a blacklist of functions. By changing the only word in this comment, it shares no words with any of those former issues!

Copy link
Copy Markdown
Member

@charris charris Jun 24, 2020

Choose a reason for hiding this comment

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

That is highly inappropriate Chuck, kind of upside down.

It doesn't look that way to me. In any case, what we are seeing here is politics entering into numpy development, and I want to avoid that. I'd also have to live with my cowardice if I didn't speak up, and I want to avoid that also.

Copy link
Copy Markdown
Author

@erfannariman erfannariman Jun 25, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

Didn't mean to bring politics into numpy development, I think that's a bit exaggerate. My only goal was to submit a PR to make the numpy devs aware of these changes that other open source projects are making (didn't explicitly create an issue for this). Besides that I think it's perfectly fine to speak out what you think and I hope we can keep doing that, just not sure if a PR would be the best place. Thanks for your input anyway :) @charris

mattip
mattip reviewed Jun 24, 2020
View reviewed changes
Comment thread .travis.yml Outdated
@mattip
Copy link
Copy Markdown
Member

mattip commented Jun 24, 2020

Language changes. I too prefer the more descriptive terms but understand it may be taken the wrong way. I am going to merge this since it really is not worth the back-and-forth.

@mattip mattip merged commit 4606d02 into numpy:master Jun 24, 2020
@SplitInfinity SplitInfinity mentioned this pull request Jul 21, 2020
Closed
64 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@charris charris charris left review comments

@eric-wieser eric-wieser eric-wieser left review comments

@mattip mattip mattip left review comments

@rgommers rgommers rgommers approved these changes

@rossbar rossbar rossbar approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

7 participants

@erfannariman @alimcmaster1 @rossbar @charris @rgommers @mattip @eric-wieser