hibtc / cpymad

Cython binding to MAD-X
http://hibtc.github.io/cpymad/
Other
27 stars 18 forks source link

ValueError: TypeError: write() argument must be str #125

Closed michi42 closed 1 year ago

michi42 commented 1 year ago

This is a follow-up of #110. It appears that the fix implemented there is not complete, as certain versions of ipykernel (at least 6.0.2 which is installed on CERN SWAN) raise ValueError instead of TypeError - even though the error message says otherwise!

ValueError                                Traceback (most recent call last)
/tmp/ipykernel_2412/334841068.py in <module>
----> 1 cpymadx.Madx()

~/python/environments/pytrain-venv/lib/python3.9/site-packages/cpymad/madx.py in __init__(self, libmadx, command_log, stdout, history, prompt, **Popen_args)
    196                 if callable(stdout):
    197                     try:
--> 198                         stdout(b'')
    199                     except TypeError:
    200                         stdout = TextCallback(stdout)

/cvmfs/sft.cern.ch/lcg/views/LCG_103swan/x86_64-centos7-gcc11-opt/lib/python3.9/site-packages/ipykernel/iostream.py in write(self, string)
    499 
    500         if not isinstance(string, str):
--> 501             raise ValueError(
    502                 "TypeError: write() argument must be str, not {type(string)}"
    503             )

ValueError: TypeError: write() argument must be str, not {type(string)}

I guess the fix is as simple as catching ValueError as well in this check...

A workaround is to pass in stdout explicitly

from cpymad import madx as cpymadx
import sys
madx = cpymadx.Madx(stdout=cpymadx.TextCallback(sys.stdout.write))
coldfix commented 1 year ago

Hi @michi42,

thanks for reporting! Is the change in https://github.com/hibtc/cpymad/commit/748843cfed10d3d3045057a1744736305380e976 what you had in mind, and fixes the error?

Best

michi42 commented 1 year ago

Hi, thanks a lot for the quick fix - this is indeed exactly what I had in mind.

coldfix commented 1 year ago

Ok. I pushed a minor release with that change that should go online within the next hour. Let me know if problems persist.

coldfix commented 1 year ago

Ah, I noticed that an unrelated test failure has manifested. The release upload will take until later when I have fixed that. Best, Thomas