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

Cython Version #54

Closed
jakirkham opened this issue Feb 23, 2015 · 8 comments
Closed

Cython Version #54

jakirkham opened this issue Feb 23, 2015 · 8 comments

Comments

@jakirkham
Copy link

I tried building with Cython and got the exception below. I'm pretty sure this is due to using a very old version of Cython as it seems to be something they fixed ( http://trac.cython.org/ticket/542 ).

NotImplementedError("New relative imports.")

It would be nice to know what version Cython cytoolz aims to be compatible with. Could we please get a note in the Readme? If I'm just missing it, I would appreciate a helpful pointer. Thanks in advance.

@eriknw
Copy link
Member

eriknw commented Feb 23, 2015

Sure, I'll add a note.

cytoolz currently isn't tested against multiple Cython versions. The goal is to use the latest available stable version. The rationale is that Cython creates the *.c and *.h files that are then uploaded and distributed to users who install cytoolz through normal means. Cython is not required to install a release of cytoolz; only a C compiler is needed.

Of course, I would also like to be accommodating to users and contributors who would like to cythonize the source files directly. What version of Cython are you using? Is it difficult for you to upgrade Cython?

Thanks for reporting this; I'll see what I can do.

@jakirkham
Copy link
Author

Thanks for the quick response. Also, thanks for being accommodating.

Understood, I initially tried to build from source (i.e. the repo), but quickly realized that wasn't an option and noted that you were probably doing this on behalf of users. All of this seems perfectly reasonable. Admittedly, I had no trouble building the C code (from PyPI).

I'm using a pretty ancient version. I don't know if it is worth it to make sure it is compatible with my version, but as you did ask it is 0.17.1. Unfortunately, yes, it is difficult to upgrade as we build a large number of dependencies from source. Again, I don't think it is worth trying to ensure compatibility on such an old version, but I do appreciate you asking.

Just out of curiosity, what command line call are you using to build just the Cython portion or are you just building everything and removing the libraries and such afterwards?

Thanks again.

@eriknw
Copy link
Member

eriknw commented Feb 23, 2015

I can make a branch that runs a test matrix of different Cython versions, and then try to update the code so it is compatible with older versions. We don't want this in the main branch, because it will slow down testing on TravisCI significantly.

To build, I use the makefile, so I just type make inplace, which runs

$(PYTHON) setup.py build_ext --inplace --cython

When doing a release, I test the C files created by the Cython I am using on this branch:
https://github.com/eriknw/cytoolz/tree/check_my_c_files

Would you prefer that releases of cytoolz included *.pyx files as well as *.c and *.h files?

@jakirkham
Copy link
Author

I can make a branch that runs a test matrix of different Cython versions...

That's a thought. I don't know how many runs you wanted to do, but it seems one should have a fixed version and then the current one.

We don't want this in the main branch, because it will slow down testing on TravisCI significantly.

Understandably, building Cython from source is a bit time consuming. Maybe it would be worth considering wheels, particularly, for any fixed versions.

To build, I use the makefile, so I just type make inplace, which runs
$(PYTHON) setup.py build_ext --inplace --cython

Cool, thanks for the info.

When doing a release, I test the C files created by the Cython I am using on this branch:
https://github.com/eriknw/cytoolz/tree/check_my_c_files

That sounds great. How often is this kept up to date? Also, are you planning on having or moving such a branch to pytoolz/cytoolz?

Would you prefer that releases of cytoolz included *.pyx files as well as *.c and *.h files?

That's probably not a bad idea the *.pyx files are a bit more readable than the *.c files that come out.

These are all just musings on my part. If they are interesting or useful great; otherwise, feel free to disregard them. After you mentioned your C branch, I am left wondering how easy would it be to have tagged releases on GitHub based off of commits in this branch? Combining something like this with Travis' ability to deploy might be nice for you and end users like myself. ( http://docs.travis-ci.com/user/deployment/pypi/ ) ( http://docs.travis-ci.com/user/deployment/custom/ ) ( http://docs.travis-ci.com/user/build-configuration/#Using-regular-expressions ) I think with a little hacking it should be possible to get the C code built and committed via Travis to a blacklisted deploy branch or similar. Admittedly, I haven't dug into this too much, but this project seems to have done something similar with R. ( https://github.com/luckyrandom/r-deploy-git )

@jakirkham jakirkham changed the title Cython Version? Is building without Cython still an option? Cython Version Feb 24, 2015
@eriknw
Copy link
Member

eriknw commented Feb 24, 2015

fyi, looks like cytoolz will work with Cython 0.17 and later once we change relative imports to absolute imports. See:

https://travis-ci.org/eriknw/cytoolz/builds/51930777

which tests this branch

https://github.com/eriknw/cytoolz/tree/test_cython_versions

You're right: I should tag releases (I did this for toolz, but forgot to do it for cytoolz), and it would make sense to have a "release" branch in the main repository that is updated every time we do a new release.

@jakirkham
Copy link
Author

Nice! I'll try it out in the morning and let you know how it goes. I don't expect any surprises. Do you think this change can be kept?

I kind of wonder what happened in the really old versions of Cython. It seems like some unsupported features, but I don't know if it is worth exploring. Cython 0.15.* is from fall of 2011.

Well, if you did go that route, I would definitely appreciate it.

@eriknw
Copy link
Member

eriknw commented Feb 24, 2015

Closing.

I just pushed absolute imports to master, and I updated "requirements_devel.txt" so it's clear that Cython version 0.17 or later is required.

Thanks for the discussion, @jakirkham!

@eriknw eriknw closed this as completed Feb 24, 2015
@jakirkham
Copy link
Author

Sounds good. By the way, building from master worked fine for me. Likewise. Thanks for the changes, as well.

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

No branches or pull requests

2 participants