Skip to content
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

fix: under pypy, when no dict args given, kwds is non-null #467

Merged

Conversation

27rabbitlt
Copy link
Contributor

@27rabbitlt 27rabbitlt commented Dec 16, 2023

Not clearly whether it's bug on numexpr side or pypy side, but when using pypy, sometimes function Numexpr_Run(PyObject *self, PyObject *args, PyObject *kwds) gets no dictionary arguments but pointer kwds is still non-null.

So add a further check of the size of dictionary kwds. It won't affect the performance since it's only one-time check.

This should fix issue #463, although not all errors could be eliminated for pypy because sys module in pypy seems not to have attribute getrefcount which is used when testing.

@mattip
Copy link

mattip commented Dec 18, 2023

It was an incompatibility in PyPy, I fixed it in this commit

@27rabbitlt
Copy link
Contributor Author

@mgorny
Copy link
Contributor

mgorny commented Jan 5, 2024

Well, I've noted in #463, PyPy wasn't fixed after all. Perhaps we could merge this for the time being? This fixes all of ex_uses_vml test failures, and it would improve compatibility with prior PyPy versions anyway.

@27rabbitlt
Copy link
Contributor Author

27rabbitlt commented Jan 5, 2024 via email

@mattip
Copy link

mattip commented Jan 14, 2024

@27rabbitlt did you mean to reopen this?

@27rabbitlt
Copy link
Contributor Author

27rabbitlt commented Jan 18, 2024

@27rabbitlt did you mean to reopen this?

Yes, but I spent some time on this and still didn't catch up the latest situation. Does this problem still affects pypy3.10 user? Should we fix it on numexpr side?

@mgorny
Copy link
Contributor

mgorny commented Jan 19, 2024

Yes, it still happens with the latest PyPy version and FWICS we haven't really figured out how to fix it on PyPy end. So yes, a workaround on numexpr side would be much appreciated.

@27rabbitlt 27rabbitlt reopened this Jan 19, 2024
@mgorny
Copy link
Contributor

mgorny commented Jan 25, 2024

Could you merge it? I can confirm it fixes the issue for me. I'm going to file a PR for the other problem (sys.getrefcount()).

@FrancescAlted FrancescAlted merged commit bec47c7 into pydata:master Jan 25, 2024
8 checks passed
@FrancescAlted
Copy link
Contributor

Thanks @27rabbitlt, @mattip and @mgorny

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.

4 participants