Boost C++ Libraries: Ticket #2744: boost::python does not support enums with duplicated values.

attachment set

hugo.lima@… — Tue, 10 Feb 2009 16:59:46 GMT

attachment → enum_with_duplicated_values.patch

Patch to fix the problem

type changed

Dave Abrahams — Tue, 10 Feb 2009 17:14:19 GMT

type Bugs → Patches

Are you saying that changing the code to use tuples even when there are no duplicates would break backward compatibility with existing user code? How so?

hugo.lima@… — Tue, 10 Feb 2009 19:29:48 GMT

The code:

for x in SomeEnum.values:

# do something with x, because I know that it is a int print x+2

because the values attribute is public.

Dave Abrahams — Tue, 10 Feb 2009 19:39:14 GMT

Then maybe a better solution is to add a public "names" dict that maps name -> value?

hugo.lima@… — Tue, 10 Feb 2009 19:59:01 GMT

Yeah, but I did not want to change the API or the current behaviour of the current boost::python based bindings with this patch. IMHO, values attribute should be a dict that maps name->value. But this will change the current API of all enums generated by boost python.

Adding a new "name" attribute without any real usecase may only increase the memory footprint with this dict. So, not changing the API and fixing a bug is good enough for now :-). What you think about?

status changed

Dave Abrahams — Tue, 10 Feb 2009 21:47:33 GMT

status new → assigned

?? The usecase is supporting your enum with multiple names for the same value

The memory footprint of this additional dict will be tiny.

The problem with the change as proposed is that it can break user code, if users are counting on "values" containing single int values, because the person using an enum is not always the same as the person exporting it.

hugo.lima@… — Wed, 11 Feb 2009 12:06:38 GMT

I can leave the values attribute intact and create another temporary attribute, just to help the enum declaration, like the current values attribute is used, and remove then after the declaration of all enums. what you think?

Results:

Do not change the current API, so do not break user code, values map to a single enum_type ever, not a tuple.
We have the bug fixed.
The same memory footprint.

Dave Abrahams — Wed, 11 Feb 2009 14:57:13 GMT

No, what I want is to increase the memory footprint by one dict per enum declaration. The names attribute would be useful in general.

hugo.lima@… — Wed, 11 Feb 2009 17:23:35 GMT

Ok, so I must keep values attribute as is and add the names attribute?

Dave Abrahams — Wed, 11 Feb 2009 18:19:31 GMT

That's the change that makes sense to me

anonymous — Wed, 11 Feb 2009 21:08:24 GMT

Ok, I'll create a new patch and post it here until next weekend, because I'm too busy now

attachment set

hugo.lima@… — Wed, 11 Feb 2009 22:41:05 GMT

attachment → enum_with_duplicated_values_v2.patch

Create a new attribute called "names" that maps the name of enum to their value and fix the "enum with duplicated values" bug.

hugo.lima@… — Wed, 11 Feb 2009 23:33:06 GMT

patch done, any comments?

Dave Abrahams — Thu, 12 Feb 2009 15:17:26 GMT

According to http://trac.edgewall.org/ticket/8047#comment:2 your patch is ill-formed. Could you please replace it with a well-formed one?

attachment set

hugo.lima@… — Thu, 12 Feb 2009 16:23:38 GMT

attachment → enum_with_duplicated_values_v2.2.patch

hugo.lima@… — Thu, 12 Feb 2009 16:25:36 GMT

the last version is the output of svn diff, with trailing spaces removals, blank spaces removals, etc...

hugo.lima@… — Tue, 17 Feb 2009 11:08:02 GMT

any comments?

Dave Abrahams — Sun, 15 Mar 2009 03:57:57 GMT

Replying to hugo.lima@openbossa.org:

any comments?

Looks great, but before I can check it in I need a testcase that fails unless this patch is applied. Do you think you could whip one up? A patch to the existing enum test case would be fine.

hugo.lima@… — Tue, 17 Mar 2009 17:29:42 GMT

Ok, I'll do it.

attachment set

hugo.lima@… — Tue, 17 Mar 2009 18:50:51 GMT

attachment → enum_with_duplicated_values_unittest.patch

Patch to the unit test

hugo.lima@… — Tue, 17 Mar 2009 19:24:53 GMT

Patch to the unit test added, it will fail because color.blood will overwrite color.red. the test just do not test if the blood has been declared.

Dave Abrahams — Tue, 17 Mar 2009 20:20:31 GMT

The new test seems to fail even after application of the patch v2.2:

running...
**********************************************************************
File "enum.py", line 7, in __main__
Failed example:
    identity(color.red)
Expected:
    enum_ext.color.red
Got:
    enum_ext.color.blood
**********************************************************************
File "enum.py", line 16, in __main__
Failed example:
    identity(color(1))
Expected:
    enum_ext.color.red
Got:
    enum_ext.color.blood
**********************************************************************
File "enum.py", line 30, in __main__
Failed example:
    identity(red)
Exception raised:
    Traceback (most recent call last):
      File "/System/Library/Frameworks/Python.framework/Versions/2.5/lib/python2.5/doctest.py", line 1212, in __run
        compileflags, 1) in test.globs
      File "<doctest __main__[8]>", line 1, in <module>
        identity(red)
    NameError: name 'red' is not defined
**********************************************************************
File "enum.py", line 44, in __main__
Failed example:
    c.x
Expected:
    enum_ext.color.red
Got:
    enum_ext.color.blood
**********************************************************************
1 items had failures:
   4 of  20 in __main__
***Test Failed*** 4 failures.

Note that you can replace existing patches so obsolete ones don't pile up in the ticket.

attachment set

hugo.lima@… — Wed, 18 Mar 2009 21:05:03 GMT

attachment → enum_with_duplicated_values_v2.3.patch

Patch to enum.cpp and their unit tests

anonymous — Wed, 18 Mar 2009 21:10:41 GMT

I dont have enough privileges to replace existing patches, maybe if I create a trac account.

I fixed the original patch, so enums are not integers and the tests do not fail. Just one issue, when I cast something to a enum, if the enum value is duplicated, it will ever return the last declared value. so I replaced the color.red by color.blood in the python test script. But that's not a problem, because color.red == color.blood and hash(color.red) == hash(color.blood).

anderson.lizardo@… — Mon, 23 Mar 2009 19:20:10 GMT

Any other comments on the latest patch (v2.3) ? I'm currently using it and it seems to work without problems.

hugo.lima@… — Fri, 08 May 2009 14:24:50 GMT

ping

status changed; resolution set

Dave Abrahams — Fri, 05 Jun 2009 21:18:16 GMT

status assigned → closed
resolution → fixed

(In [53660]) Allow duplicate enum values. Fixes #2744 Thanks to hugo.lima@…