-
-
Notifications
You must be signed in to change notification settings - Fork 18.5k
BUG: weird interaction between pyslurm, ujson that changes function signature of ujson.dumps #33877
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
Comments
@jbrockmendel do you think it's worth it to also ping the pyslurm maintainers to see what they think about this? Quite perplexed... |
@WillAyd might have a better idea on this one |
I am not familiar with pyslurm at all so can't really speak to that, but my guess is that this may be a result of ujson (and potentially pyslurm) still using single phase module initialization. We switched our internal ujson implementation over to multi-phase in #30805 but ujson itself on master still uses single phase You can read more about the differences in PEP 489 |
Thanks @WillAyd, that's very helpful indeed, I suspected it might have something to do with the intricacies of Python loading C extensions in some way but I didn't even know what to Google. I saw that Seems like it shouldn't be that difficult to convert upstream |
Cool! Yea I think for ujson you can pretty much do what was done in #30805 but on that project. After being inactive for quite a few years, there is a new maintainer of that who should be able to help get that in and released If building pyslurm for sure try having a newer Cython version. If they distribute a wheel you could also ask those maintainers to do so during the wheel building process |
Thanks @WillAyd I was about to try it out right this moment and realized the new version is out already :D |
Unfortunately, I just tried it out with the new
I wonder if this could be a bug on the Python interpreter side, even? |
We've changed the name from dumps to ujson_dumps. Might that solve this? |
I have checked that this issue has not already been reported.
I have confirmed this bug exists on the latest version of pandas.
(optional) I have confirmed this bug exists on the master branch of pandas.
Hi maintainers, I'd like to start with saying I'm not entirely sure if this is a
pandas
bug, but seeing thatpandas
bundles its ownujson
, figured I'd ask here.Problem description
TL;DR: There seems to be a very strange behavior in
ujson
depending on how it is imported (in particular, depending on whether thepyslurm
library is imported first). The observed behavior is that this modifies the function signature ofujson.dumps
(by removing thesort_keys
argument), as shown in the below examples. I'm not sure if this behavior is specific to importingpyslurm
in the beginning or not, but this is just one specific example that can be used to reproduce the problem.First, installation of these libraries via
pip
is needed:This is the script with the error:
If you run the script above, you'll see the output is:
However, if you remove the
pyslurm
import, the problem goes away!Also, reordering the imports somehow fixes the problem:
Expected Output
Importing
pyslurm
andpandas
shouldn't affect the behavior of myujson
library. Also, import orders shouldn't cause this bug.Additional Info
It seems that the very specific import order that causes this is
pyslurm -> pandas -> ujson
.I suspect this has something to do with Python's mechanism for importing C extensions, but I don't understand the internals enough to be able to debug this further.
Output of
pd.show_versions()
INSTALLED VERSIONS
commit : None
python : 3.8.2.final.0
python-bits : 64
OS : Linux
OS-release : 4.15.0-43-generic
machine : x86_64
processor : x86_64
byteorder : little
LC_ALL : None
LANG : en_US.UTF-8
LOCALE : en_US.UTF-8
pandas : 1.0.3
numpy : 1.18.3
pytz : 2020.1
dateutil : 2.8.1
pip : 20.0.2
setuptools : 46.1.3.post20200330
Cython : 0.29.17
pytest : None
hypothesis : None
sphinx : None
blosc : None
feather : None
xlsxwriter : None
lxml.etree : None
html5lib : None
pymysql : None
psycopg2 : None
jinja2 : None
IPython : None
pandas_datareader: None
bs4 : None
bottleneck : None
fastparquet : None
gcsfs : None
lxml.etree : None
matplotlib : None
numexpr : None
odfpy : None
openpyxl : None
pandas_gbq : None
pyarrow : None
pytables : None
pytest : None
pyxlsb : None
s3fs : None
scipy : None
sqlalchemy : None
tables : None
tabulate : None
xarray : None
xlrd : None
xlwt : None
xlsxwriter : None
numba : None
The text was updated successfully, but these errors were encountered: