Skip to content
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.

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

SKIP: gray vs grey #2604

Closed
sciunto opened this issue Apr 9, 2017 · 14 comments · Fixed by #5385 or #5386
Closed

SKIP: gray vs grey #2604

sciunto opened this issue Apr 9, 2017 · 14 comments · Fixed by #5385 or #5386
Assignees
Labels
🔽 Deprecation Involves deprecation 📜 type: API Involves API change(s) 💬 Discussion
Milestone

Comments

@sciunto
Copy link
Member

sciunto commented Apr 9, 2017

Description

Recently, I saw a student stuck after typing

io.imread('image.jpg', as_gray=True)

Silently, it loads as a RGB image.

Indeed, the API is as_grey. The problem is that our API is not homogeneous. It seems that US spelling is in majority in our library. I would suggest to either:

  • make sure that both gray and grey are working
  • raise a warning

Any suggestion @scikit-image/core ?

@jni
Copy link
Member

jni commented Apr 9, 2017

What does RFC mean for those of us that are uninitiated? =)

My preference is for both to work. (I think this is what we do with rgb2grey/rgb2gray?)

@soupault soupault changed the title RFC: gray vs grey SKIEP: gray vs grey Apr 9, 2017
@soupault
Copy link
Member

soupault commented Apr 9, 2017

Is seems that skimage.io is the only place where as_gray keyword is used.
It won't be a bad idea to support both as_gray and as_grey there. 👍

@sciunto
Copy link
Member Author

sciunto commented Apr 9, 2017

Request For Comments, but I have no idea what SKIEP means :D

@soupault
Copy link
Member

soupault commented Apr 9, 2017

@sciunto It is our new brand for labelling strategic discussions: SciKit-Image Enhancement Proposal (copy from PEP) 😄 .

@sciunto
Copy link
Member Author

sciunto commented Apr 9, 2017

needs a little TM them :)

@sciunto
Copy link
Member Author

sciunto commented Apr 15, 2017

Is it trivial to support as_gray along with as_grey? The **kwarg is used for the plugins... it doesn't seem nice to me to go that way. I would prefer to follow a deprecation cycle. Do you agree?

@sciunto
Copy link
Member Author

sciunto commented Apr 15, 2017

I notice that in mrphology, we have grey.py and greyreconstruct.py. Ndimage uses grey as well...

@soupault
Copy link
Member

matplotlib and Pillow use gray, grayscale.

@sciunto Sorry, I didn't catch your second proposal.

@sciunto
Copy link
Member Author

sciunto commented Apr 15, 2017

What I meant is:

  • solution 1: we use **kwargs to catch as_gray if as_grey is not set, but **kwargs is used for plugins, so it's not very clean.
  • solution 2: we add a new option as_gray with as_grey=None. If as_grey is not None, raise a warning for deprecation.

I prefer solution 2. However, if we go this way (ie in favour of gray), we have to change a lot of stuffs.

@alexdesiqueira
Copy link
Member

I'd go with as_gray, since this is the "most used" nomenclature in books, papers, et al. It could confuse some users (I was one of them). For me, @sciunto's solution 2 is the way to go.

@soupault
Copy link
Member

soupault commented May 8, 2018

@sciunto is there anything else you would like to change?

@soupault soupault modified the milestones: 0.14, 0.15 May 22, 2018
@sciunto
Copy link
Member Author

sciunto commented May 23, 2018

I just opened a small PR #3098 but ideally, it would be good to replace other places but deprecations are needed... :(

@PinkFloyded
Copy link
Contributor

As a new user of skimage it seems very strange to keep two versions of the API whenever there is a gray. This pollutes the API surface and for no good reason. I think it makes sense to choose one, deprecate the other and remove it in a major version release.

@jni
Copy link
Member

jni commented Nov 24, 2018

@PinkFloyded I actually agree with you and disagree with past me. 😂 🤷‍♂️ The intention going forward is to follow US convention everywhere. However, this will take time as it requires breaking changes and deprecations. (Not least in graycoprops, which looks weird to me spelled that way. Anyone else?)

@sciunto sciunto changed the title SKIEP: gray vs grey SKIP: gray vs grey Apr 8, 2020
@sciunto sciunto added the 🔽 Deprecation Involves deprecation label Apr 8, 2020
@grlee77 grlee77 added the 📜 type: API Involves API change(s) label Jan 25, 2021
@grlee77 grlee77 added this to To Do in skimage2 API Jan 27, 2021
psychelzh added a commit to psychelzh/caffe-wsl that referenced this issue Apr 11, 2021
@grlee77 grlee77 moved this from To Do to In progress in skimage2 API May 11, 2021
@grlee77 grlee77 moved this from In progress to Done in skimage2 API Jun 21, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment