Roundup Robot added the comment:
New changeset ae1a7c420f08 by Ethan Furman in branch 'default':
Close #18264: int- and float-derived enums now converted to int or float.
http://hg.python.org/cpython/rev/ae1a7c420f08
--
nosy: +python-dev
resolution: - fixed
stage: patch review -
Ethan Furman added the comment:
Hopefully the final patch. :)
--
Added file: http://bugs.python.org/file31172/issue18264.stoneleaf.04.patch
___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue18264
Ethan Furman added the comment:
Forgot to back out core dump tests before creating previous patch. This one
even passes the tests. :/
--
Added file: http://bugs.python.org/file31173/issue18264.stoneleaf.05.patch
___
Python tracker
Eli Bendersky added the comment:
LGTM now. Make sure to run the test(s) in refleak mode and let's wait for a
couple of days before committing, in case someone else has comments.
--
___
Python tracker rep...@bugs.python.org
Ethan Furman added the comment:
I'll plan on committing no sooner than Friday unless somebody else has
comments/corrections.
Thanks, Eli.
--
___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue18264
Ethan Furman added the comment:
Eli Bendersky added the comment:
Make sure to run the test(s) in refleak mode . . .
How extensive should testing be?
I plan on always running the refleak mode tests (now that I know how ;) .
Should I also run non-refleak tests?
Should I run tests with a
Eli Bendersky added the comment:
IMHO it's very much dependent on the change. When making C code changes, I
usually run the relevant tests with refleaks, and then run the whole test
suite; all of this --with-pydebug. Personally I've not encountered cases where
non-debug builds failed where
Ezio Melotti added the comment:
I plan on always running the refleak mode tests (now that I know how ;)
FWIW `make patchcheck` should remind you about that.
--
stage: - patch review
type: - enhancement
___
Python tracker rep...@bugs.python.org
Ethan Furman added the comment:
I don't understand.
Type checks are already performed throughout the code (int, float, True, False,
NaN, Inf, etc.).
What will opereator.index buy us?
--
___
Python tracker rep...@bugs.python.org
Nick Coghlan added the comment:
The two isinstance checks that bother me are the ones for int and float.
However, given that the JSON serialiser *already* includes those explicit
checks, I now think it makes sense to just do the minimal fix of coercing
subclasses to the base type for both of
Eli Bendersky added the comment:
I also think that exchanging the explicit type checks by __index__ merits more
thought and is outside the scope of this local fix. The proposed patch does not
add any new type checks, but acts within the bounds of code for which the type
is already
Ethan Furman added the comment:
This patch handles both float and int subclasses, and includes
fixes/improvements from the review.
--
Added file: http://bugs.python.org/file31155/issue18264.stoneleaf.02.patch
___
Python tracker
Ethan Furman added the comment:
Forgot to add float tests. Included in this patch.
--
Added file: http://bugs.python.org/file31156/issue18264.stoneleaf.03.patch
___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue18264
Ethan Furman added the comment:
Eli, your method is good. I thought I had tried something similar but I
obviously had the wrong PyLong constructor.
I'll get it implemented.
--
___
Python tracker rep...@bugs.python.org
Eli Bendersky added the comment:
Yes, AFAIU PyNumber_Long is the equivalent of Python-level int(obj). With other
constructors of PyLong you are limited by long long (while Python integers may
be arbitrarily large).
Ethan - If you're still short on time I can pretty up this patch and put it
Ethan Furman added the comment:
Thanks for the offer, Eli, but I almost have the tests done. :)
--
___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue18264
___
Ethan Furman added the comment:
Okay, patch attached with C code (thanks, Eli!), more python code, and some new
tests.
Only the `int` case is handled.
--
Added file: http://bugs.python.org/file31141/issue18264.stoneleaf.01.patch
___
Python tracker
Eli Bendersky added the comment:
Posted a Rietveld code review
--
___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue18264
___
___
Python-bugs-list
Nick Coghlan added the comment:
It occurs to me that operator.index() (without a preceding type check) is
likely the more ducktyping friendly option here.
--
___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue18264
Eli Bendersky added the comment:
I've been reading the discussion again to figure out what we need to move
forward; it appears that a simple approach of using str(int(obj)) in json
encoding when isinstance(obj, int) was acceptable for many of the participants,
and it helps solve the most
Eli Bendersky added the comment:
[please note that the patch is POC/hacky at best - it's likely to leak memory
and be careless and incomplete in other ways. I'm just trying to demonstrate
the approach]
--
___
Python tracker rep...@bugs.python.org
Ethan Furman added the comment:
I'll check you patch later against big numbers (which is where I had
difficulties). If it works I'll try to make it more complete. If it doesn't,
I've been working on just extraction the Enum member's value and using that
(works fine on the Python side ;) .
Ethan Furman added the comment:
I don't think my idea of always extracting .value is grandiose (and didn't take
that long to implement on the Python side), but it is definitely forcing me to
learn more about C and how Python is put together.
It this point I have successfully imported Enum
Changes by Giampaolo Rodola' g.rod...@gmail.com:
--
nosy: +giampaolo.rodola
___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue18264
___
___
Changes by Chris Rebert pyb...@rebertia.com:
--
nosy: +cvrebert
___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue18264
___
___
Python-bugs-list
Guido van Rossum added the comment:
The proposal to change json from using repr() to str() has unknown dangers.
I don't want the str() of IntEnum to return just the decimal string (e.g.
42), since that breaks half of the usefulness of using the enum in the first
place -- people will write
Guido van Rossum added the comment:
Modifying json to use str(int(value)) (if it detects isinstance(value, int)) is
fine with me too.
--
___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue18264
Guido van Rossum added the comment:
And similar for floats, really.
--
___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue18264
___
___
Ethan Furman added the comment:
I'd be in favor of the __protocol__ route first and the PEP 443 route second.
The problem with just tacking in `str(int(value))` or `str(float(value))` is
where does it stop? StrEnum, TupleEnum, BytesEnum, ComplexEnum,
SomeOtherInterestingConstantEnum, etc.,
Nick Coghlan added the comment:
Can I vote for something like __builtin__ as the protocol, rather than
something entirely specific to serialisation? As in return the most
appropriate builtin type with the same value? Then a converter
(operator.builtin?) could coerce builtin subclasses to their
Guido van Rossum added the comment:
Hm. Then I prefer just calling the appropriate builtin, e.g. int().
--Guido van Rossum (sent from Android phone)
On Jun 21, 2013 6:08 PM, Nick Coghlan rep...@bugs.python.org wrote:
Nick Coghlan added the comment:
Can I vote for something like __builtin__
Barry A. Warsaw added the comment:
On Jun 22, 2013, at 01:08 AM, Nick Coghlan wrote:
Can I vote for something like __builtin__ as the protocol, rather than
something entirely specific to serialisation? As in return the most
appropriate builtin type with the same value? Then a converter
Eli Bendersky added the comment:
On Fri, Jun 21, 2013 at 6:59 PM, Barry A. Warsaw rep...@bugs.python.orgwrote:
Barry A. Warsaw added the comment:
On Jun 22, 2013, at 01:08 AM, Nick Coghlan wrote:
Can I vote for something like __builtin__ as the protocol, rather than
something entirely
Guido van Rossum added the comment:
Change json to call int() first.
--Guido van Rossum (sent from Android phone)
On Jun 21, 2013 7:45 PM, Eli Bendersky rep...@bugs.python.org wrote:
Eli Bendersky added the comment:
On Fri, Jun 21, 2013 at 6:59 PM, Barry A. Warsaw rep...@bugs.python.org
Ethan Furman added the comment:
On 06/21/2013 07:49 PM, Guido van Rossum wrote:
Eli Bendersky added the comment:
Practically speaking, what should be done to make enum play well with JSON
without writing new PEPs? I think we still want to convert those stdlib
constants to IntEnums...
Guido van Rossum added the comment:
Yes for float() -- but for str() it would seem redundant? (Or what's
the context?)
On Fri, Jun 21, 2013 at 8:23 PM, Ethan Furman rep...@bugs.python.org wrote:
Ethan Furman added the comment:
On 06/21/2013 07:49 PM, Guido van Rossum wrote:
Eli Bendersky
Ethan Furman added the comment:
Guido van Rossum added the comment:
Yes for float() -- but for str() it would seem redundant? (Or what's
the context?)
If a user has
class Color(StrEnum):
red = 'ff'
green = '00ff00'
blue = 'ff'
..
..
..
oh. `str()`
Nick Coghlan added the comment:
Whatever we do needs to be something third party serialisation libraries
can also adopt with minimal compatibility risk for older versions of Python.
Yes, that serialisation will lose the new debugging information. That's
fine - if people want to map from a
New submission from Nick Coghlan:
Replacing an integer constant with the current incarnation of enum.IntEnum
breaks JSON serialisation:
from enum import Enum
from enum import Enum, IntEnum
class Example(IntEnum):
... x = 1
...
import json
json.dumps(1)
'1'
json.loads(json.dumps(1))
Changes by Barry A. Warsaw ba...@python.org:
--
nosy: +barry
___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue18264
___
___
Python-bugs-list mailing
Barry A. Warsaw added the comment:
On Jun 19, 2013, at 01:45 PM, Nick Coghlan wrote:
Allowing __str__ to be inherited from Enum rather than from the concrete type
will similarly break any serialisation protocol that relies on str() to
handle integers. The float case is even trickier, since
Eric Snow added the comment:
It's for times like this that I wonder if a simple serialization protocol might
be worth it--something separate from __str__() but much simpler than the pickle
protocol. __str__() could be the fallback. In the end, though, I always
conclude that it's not worth
Amaury Forgeot d'Arc added the comment:
in encoder.py:
...
elif instance(value, int):
yield buf + str(value)
...
What if we use int(str(value)) instead?
--
nosy: +amaury.forgeotdarc
___
Python tracker rep...@bugs.python.org
Eric Snow added the comment:
Serialization isn't the only issue - you have to know how
to deserialize as well.
+1
(TBH, I wish the json module had better hooks for extending
both serialization and deserialization of non-basic types.)
+1
There's at least one stdlib module that does this
Ethan Furman added the comment:
Nick, in your example it looks like json is using the __str__ for int, but the
__repr__ for float -- is this correct?
--
assignee: - ethan.furman
nosy: +eli.bendersky, ethan.furman
___
Python tracker
Barry A. Warsaw added the comment:
On Jun 19, 2013, at 02:09 PM, Eric Snow wrote:
There's at least one stdlib module that does this pretty well (sqlite3, I
think). This is where a simple serialization (and deserialization of course)
protocol would come in handy.
Yeah, my database layer also
Antoine Pitrou added the comment:
What if we use int(str(value)) instead?
You mean str(int(value)). Sounds good to me.
--
nosy: +pitrou
___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue18264
Antoine Pitrou added the comment:
Serialization isn't the only issue - you have to know how to
deserialize as well.
I think this is pretty much besides the point. IntEnums are meant to be
substitutible with plain ints, so you can deserialize as a plain int.
Moreoven, JSON doesn't fill the
Nick Coghlan added the comment:
While I agree with forcing int subclasses to true integers in the JSON module,
that may not be enough - the problem will affect third party serialisers as
well. Whiel the debugging gains won't be as high, we may need to override
__str__() in enum.IntEnum to
Ethan Furman added the comment:
I have no problem with leaving __str__ as the inherited type's, and just
keeping the __repr__ as the enum add-on; this could be one of the differences
between a pure Enum and a hybrid Enum.
Is it safe to change the json behaviour back to using __str__ for
Ethan Furman added the comment:
Eric,
The pickle support is solely in Enum. No changes were made to pickles.
--
___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue18264
___
Ethan Furman added the comment:
I was unable to find any references to previous problems with json and floats.
A quick-n-dirty patch yields the following:
-- from json import dumps, loads
-- from enum import Enum
-- class FE(float, Enum):
... pass
...
-- class Test(FE):
... one = 1.0
...
Changes by Ethan Furman et...@stoneleaf.us:
--
nosy: +ezio.melotti, rhettinger
___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue18264
___
___
Ethan Furman added the comment:
Here's the relevant routine from _json.c:
-
static PyObject *
encoder_encode_float(PyEncoderObject *s, PyObject *obj)
{
/* Return the JSON representation of a PyFloat */
double i = PyFloat_AS_DOUBLE(obj);
if
54 matches
Mail list logo
