Skip to content

Use enums instead of manually assigning small integers #1243

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

Closed
ddfisher opened this issue Feb 25, 2016 · 10 comments
Closed

Use enums instead of manually assigning small integers #1243

ddfisher opened this issue Feb 25, 2016 · 10 comments
Labels

Comments

@ddfisher
Copy link
Collaborator

We use manually rolled enum equivalents in a number of places instead of just using the 3.4 enum module. Using real enums would make debugging a lot nicer in some cases.

The main downside to this change is that it'll add an additional dependency when running on 3.3 and below.

@JukkaL JukkaL added the refactoring Changing mypy's internals label Mar 1, 2016
@JukkaL
Copy link
Collaborator

JukkaL commented Mar 1, 2016

This would be a reasonable thing to refactor, though not high priority.

@gnprice gnprice modified the milestone: Future Mar 1, 2016
@elazarg
Copy link
Contributor

elazarg commented Sep 5, 2016

I have a working branch, with 4 new Enums.

By order of impact:

  1. Literal(YES|NO|TYPE): relatively minor and local change
  2. Variance: The result is longer and not much clearer, except here and there in type annotations
  3. Argument kinds
  4. Node kinds

@gvanrossum
Copy link
Member

Maybe do one at a time? Do you use enum34 from PyPI or would this imply dropping Python 3.3?

@elazarg
Copy link
Contributor

elazarg commented Sep 5, 2016

I did it only as an experiment. the Literal part is pretty trivial (and not very helpful). I'm not familiar with enum34; I can try it.

@elazarg
Copy link
Contributor

elazarg commented Sep 5, 2016

As for Variance, I think that NewType will be more suitable than Enum, because the names are already very descriptive, and it's unlikely to be confused wit something else. Node kinds and Argument kinds are more likely to be mixed accidentally,

(I used IntEnum to avoid dealing with Json serialization. I guess it's not big deal anyway)

@elazarg
Copy link
Contributor

elazarg commented Jan 9, 2017

I don't know if that's common knowledge, but I've found out that enum access is very very slow. I guess it won't be noticeable in this project, but that's another thing to take into account.

@gvanrossum gvanrossum removed this from the Future milestone Mar 29, 2017
@JukkaL
Copy link
Collaborator

JukkaL commented Oct 2, 2017

I've heard bad things about enum performance. Improving mypy performance is going to be a main focus of the core team, and switching to enum carries some risk. It's not worth it, even if the risk is small.

I'm not closing this issue since this may make sense in the long term, after we are happy with mypy performance.

@elazarg
Copy link
Contributor

elazarg commented Oct 2, 2017

How about using NewType?

@JukkaL
Copy link
Collaborator

JukkaL commented Oct 2, 2017

NewType creates a subtype so it's less safe than an enum. I'd prefer to (eventually) switch to enums.

@ilevkivskyi
Copy link
Member

There is a discussion on python-dev about improving Python start-up time and enums being slow is one of the issues discussed. There is a chance performance will be improved in Python 3.7.

@JukkaL JukkaL changed the title use enums instead of manually assigning small integers Use enums instead of manually assigning small integers May 18, 2018
@hauntsaninja hauntsaninja closed this as not planned Won't fix, can't repro, duplicate, stale Apr 26, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

7 participants