GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — use enum name in repr by koubaa · Pull Request #2239 · pythonnet/pythonnet

Conversation

@koubaa

@koubaa koubaa commented Sep 18, 2023 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Contributor

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

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
Mohamed Koubaa added 3 commits September 18, 2023 07:49
Mohamed Koubaa
use enum name in repr
d9c326c
Mohamed Koubaa
update test
3acd1ca
Mohamed Koubaa
changelog
be2ae67
This was referenced Sep 18, 2023
Enum repr ansys/ansys-pythonnet#8
Merged
[Embedding] Enum values are integers, they should be string-based for usability ansys/pymechanical#240
Closed
@lostmsu

lostmsu commented Sep 18, 2023 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Member

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.
heartacker and koubaa reacted with thumbs up emoji
All reactions
  • 👍 2 reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
Mohamed Koubaa added 2 commits September 19, 2023 09:36
Mohamed Koubaa
Use python convention
dea66fb
Mohamed Koubaa
fix test
a3be481
@koubaa

koubaa commented Sep 19, 2023

Copy link
Copy Markdown
Contributor Author

@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?
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@lostmsu

lostmsu commented Sep 19, 2023

Copy link
Copy Markdown
Member

@koubaa both of course.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
lostmsu
lostmsu requested changes Sep 19, 2023
View reviewed changes
Comment thread src/runtime/Types/ClassObject.cs Resolved

Uh oh!

There was an error while loading. Please reload this page.
Comment thread src/runtime/Types/ClassObject.cs Resolved

Uh oh!

There was an error while loading. Please reload this page.
Mohamed Koubaa added 2 commits September 21, 2023 10:40
Mohamed Koubaa
respect base enum type
5133cef
Mohamed Koubaa
fix other assert
6e70b0e
@koubaa

koubaa commented Sep 21, 2023

Copy link
Copy Markdown
Contributor Author

@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
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@lostmsu

lostmsu commented Sep 21, 2023 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Member

@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()!;
}
koubaa reacted with heart emoji
All reactions
  • ❤️ 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
lostmsu
lostmsu reviewed Sep 21, 2023
View reviewed changes
Comment thread src/runtime/Types/ClassObject.cs Resolved

Uh oh!

There was an error while loading. Please reload this page.
Mohamed Koubaa
refactor
2274595
@lostmsu
lostmsu merged commit 22d07dd into pythonnet:master Sep 22, 2023
koubaa added a commit to ansys/ansys-pythonnet that referenced this pull request Sep 25, 2023
@koubaa
Enum repr (#8)
5030f75 
mirror of upstream PR:
pythonnet/pythonnet#2239
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@lostmsu lostmsu lostmsu requested changes

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Enum __repr__ string could be improved

Uh oh!

There was an error while loading. Please reload this page.

2 participants

@koubaa @lostmsu
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.