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

Rebuild C files using cython==0.29.28 #187

Merged
merged 2 commits into from
Mar 11, 2022

Conversation

PeterJCLaw
Copy link
Contributor

@PeterJCLaw PeterJCLaw commented Mar 11, 2022

This seems to be needed to get CI to pass and presumably adds support for more recent Python versions.

Generated using the same lines as .travis/test_cython_files.sh does, on Python 2.7 3.10.

I'm not personally a Cython expert so I'm not really able to comment on the actual changes here.

This seems to be needed to get CI to pass and presumably adds support
for more recent Python versions.
@PeterJCLaw PeterJCLaw mentioned this pull request Mar 11, 2022
@PeterJCLaw
Copy link
Contributor Author

Meta: is updating these files for every version of Cython the expected approach? Would it make sense to pin to a (minor) version of Cython and perhaps have the test know how to ignore patch-level version number changes?

Not sure why there should be whitespace changes as a result of a
different Python version, but there are. Since CI only validates
these on Python 3.6+, hopefully this will work on all such Pythons.
@dannyroberts
Copy link
Member

Thanks @PeterJCLaw. Yeah this is a known annoyance. We'd be happy to accept a contribution that improved it. I think basically all the relevant context is captured in these two comments #182 (comment) and #183 (comment) from the last time exactly this happened.

@dannyroberts
Copy link
Member

Since it's been a while since we did this, I confirmed that I get the same output that you (and the tests on the PR) did when I run

find jsonobject -iname '*.c' -delete
find jsonobject -iname '*.so' -delete
python setup.py build_ext --inplace

locally.

I'm going to also edit the name of this PR to include the cython version so it's more easily identifiable in the future.

@dannyroberts dannyroberts changed the title Rebuild C files using the latest Cython Rebuild C files using cython== 0.29.28 Mar 11, 2022
@dannyroberts dannyroberts changed the title Rebuild C files using cython== 0.29.28 Rebuild C files using cython==0.29.28 Mar 11, 2022
@dannyroberts
Copy link
Member

I'll also update the matrix of python versions this is tested against to only include non-deprecated versions of python

@dannyroberts dannyroberts merged commit 69d0783 into dimagi:master Mar 11, 2022
@dannyroberts
Copy link
Member

Thanks for your contribution @PeterJCLaw!

@PeterJCLaw PeterJCLaw deleted the update-cython-expectation branch March 11, 2022 20:47
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.

2 participants