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

Reimplement constant groups as Enums #70

Open
ramalho opened this issue Feb 7, 2015 · 9 comments · May be fixed by #109
Open

Reimplement constant groups as Enums #70

ramalho opened this issue Feb 7, 2015 · 9 comments · May be fixed by #109

Comments

@ramalho
Copy link
Contributor

ramalho commented Feb 7, 2015

Pingo has several groups of constants used for certain attributes such as pin mode (IN, OUT, ANALOG), pin state etc. Currently they are strings because that makes it easy to inspect their values. Ideally they should be enums, but there was a wide variety of enum implementations in Python, making it hard to choose. Since Python 3.4 has an Enum type in the standard library and it has been backported to work with Python 2 in the Enum34 project, Pingo should use it: https://pypi.python.org/pypi/enum34

@Vido
Copy link
Contributor

Vido commented Feb 8, 2015

But why? The strings are just fine.
If Python2.7 does not support native enum, why should we care?
"""Although practicality beats purity."""

@ramalho
Copy link
Contributor Author

ramalho commented Feb 8, 2015

On Sun, Feb 8, 2015 at 6:10 PM, Lucas Vido [email protected] wrote:

But why? The strings are just fine.

Because with Enums it's easy for the user and for us to verify that
the value provided is a valid one, and to inspect what are the
acceptable values.

If Python2.7 does not support native enum, why should we care?

It's not native in Python 3.4 either, it's just in the stdlib. An is a
simple requirement for Python 2.7. I've always wanted to use enums for
this purpose, the only reason we didn't do it before was that I did
not know the "canonical" Enum for Python 3.4 was backported.

The use of unverified constants is a major source of pain in libraries
that work with low-level APIs. Replacing the integer constants with
strings was a stopgap measure to make debugging easier, but still adds
no automatic checking.

"""Although practicality beats purity."""

If you quote the The reason why we should use enums is in the next
line of the Zen:

"""Errors should never pass silently."""

Cheers,

Luciano

Luciano Ramalho
Twitter: @ramalhoorg

Professor em: http://python.pro.br
Twitter: @pythonprobr

@danilobellini
Copy link
Contributor

At first, I don't see any problem. But without the __prepare__ metaclass method, which is only available in Python 3.x, we can't ensure the class namespace order. As it seems, the __order__ workaround will be needed, or perhaps a direct call to type(name, bases, namespace) with bases = (Enum,) and an OrderedDict for the namespace, or even the easier Enum(name, whitespace_separated_names) when the values doesn't matter.

I just don't like nesting. The constants like IN, OUT and ANALOG might remain there easily as the enum classes are iterables allowing assignments like IN, OUT, ANALOG = PortConfigs and LOW, HIGH = DigitalOutputs. Anyway, using assignments to names like DigitalOutputs.low (i.e. keeping FullNames.including_enum_class_names) would be bureaucratic, so I'd say typecasts from int and str are desirable. Automatic typecasts from str in assignments might even deprecate the constants like IN as these are there mostly to avoid hidden errors due to typos, however I think these should be kept as they're already a legacy, even though they wouldn't be strings anymore. Comparison using __eq__ like my_pin.value == "low" would also be possible if we let the comparison include enum_obj.name for equalness (internally to __eq__), and the people still can use the is when they want to ensure the enum object is really the same (including that both in the comparison are enum objects).

@ramalho
Copy link
Contributor Author

ramalho commented Feb 9, 2015

On Mon, Feb 9, 2015 at 1:29 AM, Danilo de Jesus da Silva Bellini
[email protected] wrote:

At first, I don't see any problem. But without the prepare metaclass method, which is only available in Python 3.x, we can't ensure the class namespace order. As it seems, the order workaround will be needed, or perhaps a direct call to type(name, bases, namespace) with bases = (Enum,) and an OrderedDict for the namespace, or even the easier Enum(name, whitespace_separated_names) when the values doesn't matter.

I understand the issues you are talking about (attribute ordering in a
class was the reason for using a metaclass in the extended edition of
my Encapsulation with Descriptors talk) but I do not understand why
you are talking about implementation details. Are you commenting on
the existing Enum34 PyPI package? Because that is what I propose we
use, and not implement our own enum. Otherwise, please elaborate.

I just don't like nesting. The constants like IN, OUT and ANALOG might remain there easily as the enum classes are iterables allowing assignments like IN, OUT, ANALOG = PortConfigs and LOW, HIGH = DigitalOutputs. Anyway, using assignments to names like DigitalOutputs.low (i.e. keeping FullNames.including_enum_class_names) would be bureaucratic, so I'd say typecasts from int and str are desirable. Automatic typecasts from str in assignments might even deprecate the constants like IN as these are there mostly to avoid hidden errors due to typos, however I think these should be kept as they're already a legacy, even though they wouldn't be strings anymore. Comparison using eq like my_pin.value == "low" would also be possible if we let the comparison include enum_obj.name for equalness (internally to < code>eq), and the people still can use the is when they want to ensure the enum object is really the same (including that both in the comparison are enum objects).

This paragraph is not clear to me either... sorry. Perhaps some code
snippets would help clarify the use cases you believe would be
negatively affected.

Luciano Ramalho
Twitter: @ramalhoorg

Professor em: http://python.pro.br
Twitter: @pythonprobr

@s-celles
Copy link
Contributor

I'm just posting here just to warm that moving from string to enum (enum34) will drive some problems with JSON serialization (probably useful to expose boards via a REST API - see https://github.com/garoa/pingo/issues/68 ).
Here is a recipe to fix it http://stackoverflow.com/questions/24481852/serialising-an-enum-member-to-json.
I'm also for enum because of completion and reflection but we must be aware of this.

@danilobellini
Copy link
Contributor

[...] but I do not understand why you are talking about implementation details [...]

These are examples, not really details. I'm talking about ways to use the Enum34 package. Some of these are part of the package API/documentation (e.g. the __order__ workaround).

Are you commenting on the existing Enum34 PyPI package?

Yes. I'm talking about different ways to use it to get the same behavior in Python 2 and 3 when using it.

This paragraph is not clear to me either [...]

All I'm talking about is about using the Enum34 PyPI package and a way to keep the "legacy" API already implemented in Pingo together with a new enum-based API.

I'm not for nor against using enums. The main advantage is code completion and reflection, which I think are nice stuff to have in Pingo, but it would be a problem if casting to/from strings gets complicated or if the code written today that depends on Pingo gets deprecated and broken at the same time.

@ramalho
Copy link
Contributor Author

ramalho commented Feb 16, 2015

On Mon, Feb 16, 2015 at 7:09 PM, scls19fr [email protected] wrote:

I'm just posting here just to warm that moving from string to enum (enum34) will drive some problems with JSON serialization (probably useful to expose boards via a REST API - see #68 ).
Here is a recipe to fix it http://stackoverflow.com/questions/24481852/serialising-an-enum-member-to-json.
I'm also for enum but we must be aware of this.

Thanks for the reminder and the link, it will be useful.


Reply to this email directly or view it on GitHub.

Luciano Ramalho
Twitter: @ramalhoorg

Professor em: http://python.pro.br
Twitter: @pythonprobr

@ramalho
Copy link
Contributor Author

ramalho commented Feb 16, 2015

On Mon, Feb 16, 2015 at 7:26 PM, Danilo de Jesus da Silva Bellini
[email protected] wrote:

I'm not for nor against using enums. The main advantage is code completion
and reflection, which I think are nice stuff to have in Pingo,

These are great reasons. I plan to start using iPython with Pingo as
soon as I finish my book (a matter of few weeks now!).

but it would
be a problem if casting to/from strings gets complicated

We already have a conversion problem to deal with: native board API
use different codes to represent modes and values. Board drivers
already have to deal with that. I believe using Enum34 will make this
problem more manageable, and easier to debug by making it clear when a
certain value is a native code or a Pingo enum value.

or if the code written today that depends on Pingo gets deprecated and broken at the same time.

Pingo is still in version 0.2.0. It's unstable and has to be. There
are many fundamental design questions still open.

We shouldn't slow down our progress because of client code written
today. Let's begin worrying about that when we get past 0.8 ;-).

Best,

Luciano

Luciano Ramalho
Twitter: @ramalhoorg

Professor em: http://python.pro.br
Twitter: @pythonprobr

@s-celles
Copy link
Contributor

@ramalho PR seems to pass unit tests.
Could you have a look ?
Thanks.
Kind regards

Sébastien

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.

4 participants