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

use enum name in repr #2239

Merged
merged 8 commits into from Sep 22, 2023
Merged

use enum name in repr #2239

merged 8 commits into from Sep 22, 2023

Conversation

koubaa
Copy link
Contributor

@koubaa koubaa commented Sep 18, 2023

What does this implement/fix? Explain your changes.

Proposal to fix #2238

Before the changes:

Python 3.11.4 (tags/v3.11.4:d2340ef, Jun  7 2023, 05:45:37) [MSC v.1934 64 bit (AMD64)] on win32
Type "help", "copyright", "credits" or "license" for more information.
>>> import clr; import System
>>> System.DayOfWeek.Monday
<System.DayOfWeek object at 0x0000024D5BAC7300>

IronPython (as a reference):

IronPython 2.7.4 (2.7.0.40) on .NET 4.0.30319.42000 (64-bit)
Type "help", "copyright", "credits" or "license" for more information.
>>> import System
>>> System.DayOfWeek.Monday
System.DayOfWeek.Monday

With these changes:

Python 3.11.4 (tags/v3.11.4:d2340ef, Jun  7 2023, 05:45:37) [MSC v.1934 64 bit (AMD64)] on win32
Type "help", "copyright", "credits" or "license" for more information.
>>> import clr; import System
>>> System.DayOfWeek.Monday
DayOfWeek.Monday

Checklist

  • Make sure to include one or more tests for your change
  • If an enhancement PR, please create docs and at best an example
  • Ensure you have signed the .NET Foundation CLA
  • Add yourself to AUTHORS
  • Updated the CHANGELOG

@lostmsu
Copy link
Member

lostmsu commented Sep 18, 2023

Can you please make the format similar to Python's enum.Enum:

>>> from enum import Enum
>>> class Color(Enum):
...   RED = 1
...   GREEN = 2
...
>>> repr(Color.GREEN)
'<Color.GREEN: 2>'

I suggest to use decimal values when enum is a regular enum, and either binary or hexadecimal when it has [Flags].

P.S. Disregard the Mac and ARM failures, they are caused by our test infra.

@koubaa
Copy link
Contributor Author

koubaa commented Sep 19, 2023

@lostmsu Done - but I'm not really that happy with the Flags case. ToString() seems to make some choices about how to represent the value. Do you mean to only show the hex value if its Flags, instead of trying to show both integer and name?

@lostmsu
Copy link
Member

lostmsu commented Sep 19, 2023

@koubaa both of course.

src/runtime/Types/ClassObject.cs Outdated Show resolved Hide resolved
src/runtime/Types/ClassObject.cs Outdated Show resolved Hide resolved
@koubaa
Copy link
Contributor Author

koubaa commented Sep 21, 2023

@lostmsu done.

The switch case isn't so pretty, but I couldn't think of a way to improve it. here's a minimal compiler explorer link with that logic if anyone wants to try and see if they can improve it: https://godbolt.org/z/dYG6sdnKo

@lostmsu
Copy link
Member

lostmsu commented Sep 21, 2023

@koubaa try using Convert.ChangeType

string ConvertFlags(Enum value) {
    Type primitiveType = value.GetType().GetEnumUnderlyingType();
    string format = "X" + (Marshal.SizeOf(primitiveType) * 2).ToString(CultureInfo.InvariantCulture);
    var primitive = (IFormattable)Convert.ChangeType(value, primitiveType);
    return primitive.ToString(format, null);
   
}

string ConvertValue(Enum value) {
    Type primitiveType = value.GetType().GetEnumUnderlyingType();
    return Convert.ChangeType(value, primitiveType).ToString()!;
}

@lostmsu lostmsu merged commit 22d07dd into pythonnet:master Sep 22, 2023
22 of 23 checks passed
koubaa added a commit to ansys/ansys-pythonnet that referenced this pull request Sep 25, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Enum __repr__ string could be improved
2 participants