alecthomas / voluptuous

CONTRIBUTIONS ONLY: Voluptuous, despite the name, is a Python data validation library.
https://pypi.org/project/voluptuous
BSD 3-Clause "New" or "Revised" License
1.82k stars 218 forks source link

Enum support broke this for py2.7, homepage still lists py2.7 as being supported... what to do? #453

Closed epenet closed 2 years ago

epenet commented 2 years ago

This broke for py2.7, homepage still lists py2.7 as being supported... what to do?

Originally posted by @jamesbraid in https://github.com/alecthomas/voluptuous/issues/450#issuecomment-1085343531

epenet commented 2 years ago

@jamesbraid does it work correctly if you install enum34 manually? https://pypi.org/project/enum34/

jamesbraid commented 2 years ago

@jamesbraid does it work correctly if you install enum34 manually? https://pypi.org/project/enum34/

yep, installing enum34 works.

possible to add that as a dependency? I locked our systems on 0.12.2 for now.

epenet commented 2 years ago

454 will resolve this (use Enum if it is available, ignore it if not)

However, running tests I have found another issue with emails: #455 that also needs resolving IMO

spacegaier commented 2 years ago

I am just wondering if this conditional Enum logic is the best approach since that means we are diverging in terms of behavior between 2.7 and >3.4, even if only for an error handling scenario.

However, the alternative would introduce a dependency which so far we managed without.

Any more opinions/votes here for either approach?

epenet commented 2 years ago

IMO this makes it most compatible: if you have Enum available it just works. If not it doesnt matter as you don't have Enum anyway.

spacegaier commented 2 years ago

Chatting with @epenet made me realize my mistake in my thinking:

We only ever reach that error scenario in voluptuous if the caller is attempting to use Coerce with Enum meaning they must have it available already, either directly via Python version or via separate dependency such as enum34. So no sense in adding it as a dependency to voluptuous 🤦‍♂️