Skip to content

More consistent handling of 'None' vs. 'none' #19300

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

Open
timhoffm opened this issue Jan 14, 2021 · 6 comments · May be fixed by #26004
Open

More consistent handling of 'None' vs. 'none' #19300

timhoffm opened this issue Jan 14, 2021 · 6 comments · May be fixed by #26004

Comments

@timhoffm
Copy link
Member

timhoffm commented Jan 14, 2021

Summary

The strings 'None' vs 'none' are not handled consistently:

  • At least MarkerStyle only accepts 'None' (xref Cleanup code for format processing #19291 (comment)). - Maybe other places too.
  • However, in many places we accept both 'None', 'none' (and sometimes also other capitalizations).
  • Counting occurences, we have 777 'none' and 139 'None' in our code base.

Proposed fix

Aim at making the API more consistent by:

  • Choose one version and use it whenever possible throughout code, docs and examples.
    • I propose to choose 'none' because most other named strings are lowercase. And it's a step further away from None, which may help making it more clear that those two are different.
  • Make all other places accept that version too.
  • For now do not deprecate other writings or conversions. If a way of writing is working now, there'll be lots of code out there using it. It's not worth breaking that.

Note @brunobeltran You might stumble over these inconsistencies when formalizing types.

@mwaskom
Copy link

mwaskom commented Jan 14, 2021

Aren't there places where they mean different things? i.e. None means "use the rcParams default and "none" means "use an appropriate null value"?

This is the case at very least for facecolor, probably others...

@timhoffm
Copy link
Member Author

timhoffm commented Jan 14, 2021

This is about capitalization of strings 'None' vs. 'none' (I'll add better formatting above). You are right that None is something completely different.

@mwaskom
Copy link

mwaskom commented Jan 14, 2021

Oops! Missed that entirely, sorry for noise.

@brunobeltran
Copy link
Contributor

I have definitely encountered this in several places (including markers). For now I've been erring on the side of accepting both "None" and "none" as synonyms if they already were used before, but only documenting that we accept 'none'. Sounds like this is completely in line with your proposal.

I'd add my vote for 'none' to also be added in places where it is not currently accepted, and documentation updated.

@oscargus
Copy link
Member

I was playing around with this and noted two things:

  1. line styles are converted into "None" as the preferred representation:
    if ls in [' ', '', 'none']:
    ls = 'None'
  2. Some code relies on that:
    # Note: We don't val.lower() != 'none' because val is not
    # necessarily a string (can be a tuple for colors). This
    # is safe, because *val* comes from _process_plot_format()
    # which only returns 'None'.

@timhoffm
Copy link
Member Author

Yes it’s a bit of a mess.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants