da4089 / simplefix

Simple FIX protocol implementation for Python
MIT License
229 stars 63 forks source link

message: Fix IntEnum tag usage #56

Closed reprographix closed 10 months ago

reprographix commented 10 months ago

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

da4089 commented 10 months ago

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.

reprographix commented 10 months ago

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 commented 10 months ago

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.

reprographix commented 10 months ago

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 commented 10 months ago

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

reprographix commented 10 months ago

Thank you!