Skip to content

message: Fix IntEnum tag usage#56

Merged
da4089 merged 5 commits intomasterfrom
unknown repository
Sep 8, 2023
Merged

message: Fix IntEnum tag usage#56
da4089 merged 5 commits intomasterfrom
unknown repository

Conversation

@ghost
Copy link

@ghost ghost commented Sep 7, 2023

IntEnum were being converted to strings in fix_tag which caused issues when sourcing FIX tags from an IntEnum.

IntEnum were being converted to strings in `fix_tag` which caused issues when sourcing FIX tags from an IntEnum.
@da4089
Copy link
Owner

da4089 commented Sep 8, 2023

Thanks for the PR!

This is a good one. I tested out the scenario, and I couldn't reproduce the issue. So I added a test case and pushed it (see commit 4aa0e72) and the CI picked up that it works in 3.11 but not earlier releases.

I'm looking at this in some more detail.

@ghost
Copy link
Author

ghost commented Sep 8, 2023

Hi,

It seems like enum behavior has indeed changed in Python 3.11+: https://mail.python.org/archives/list/python-dev@python.org/message/RN3WCRZSTQR55DOHJTZ2KIO6CZPJPCU7/

I just fixed a namespace error in my pull request.

@da4089
Copy link
Owner

da4089 commented Sep 8, 2023

Rather than the explicit check for IntEnum type, I think it might be better to check for the ability to convert to an integer, like:

if hasattr(value, '__int__'):
    return str(int(value)).encode('ASCII')

The performance is more-or-less the same, and it will hopefully support anything else that comes along that can support the int() conversion.

What do you think? If that's ok for you, could you make that change, and I'll merge it.

@ghost
Copy link
Author

ghost commented Sep 8, 2023

That would be indeed much better! I just committed the updated version to the branch, you'll likely want to squash my commits when merging.

@da4089 da4089 merged commit 0ea045b into da4089:master Sep 8, 2023
@da4089
Copy link
Owner

da4089 commented Sep 8, 2023

Thanks again for this -- much appreciated!
I've pushed v1.0.16 to PyPI containing this fix.

@ghost
Copy link
Author

ghost commented Sep 8, 2023

Thank you!

@ghost ghost deleted the intenum-tag branch September 13, 2023 13:31
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.

1 participant