pingo-io / pingo-py

Generic API for controlling boards with programmable IO pins
http://pingo.io
MIT License
257 stars 48 forks source link

Reimplement constant groups as Enums #70

Open ramalho opened 9 years ago

ramalho commented 9 years ago

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 commented 9 years ago

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

ramalho commented 9 years ago

On Sun, Feb 8, 2015 at 6:10 PM, Lucas Vido notifications@github.com 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 commented 9 years ago

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 commented 9 years ago

On Mon, Feb 9, 2015 at 1:29 AM, Danilo de Jesus da Silva Bellini notifications@github.com 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

scls19fr commented 9 years ago

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 commented 9 years ago

[...] 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 commented 9 years ago

On Mon, Feb 16, 2015 at 7:09 PM, scls19fr notifications@github.com 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 commented 9 years ago

On Mon, Feb 16, 2015 at 7:26 PM, Danilo de Jesus da Silva Bellini notifications@github.com 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

scls19fr commented 8 years ago

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

Sébastien