Skip to content

Conversation

@jf---
Copy link

@jf--- jf--- commented Nov 24, 2016

No description provided.

@zeograd
Copy link
Owner

zeograd commented Nov 24, 2016

Thanks for the PR.
Are the tests passing on your side ?
There might be some missing parts in the cython implementation needed to match the few evolution which happened in the python Line implementation

@jf---
Copy link
Author

jf--- commented Nov 24, 2016

hi @zeograd , yep, the test suite is passing...
if you look at the diff, you see that 2 attr's were added to the cython implementation

@zeograd
Copy link
Owner

zeograd commented Nov 24, 2016

Thanks for your answer.

The test suite is failing hard on my side (on python 2.7.12 under debian unstable x64) even on the most basic equality test (the rest is probably failing due to this).

I never coded in cython, but since there is a custom eq method on the PyLine class, I'd expect a similar method on the GLine implementation, which seems missing.

Have I missed something about this part ?

@jf---
Copy link
Author

jf--- commented Nov 24, 2016

@zeograd , possible the python rather than cython module was loaded when the PR was submitted.
I did ran the test successfully, so need to think of a mechanism to test both implementations...

@zeograd
Copy link
Owner

zeograd commented Nov 24, 2016

@jf--- the test before PR being against the python version is probable

Regarding having both implementations, I made local changes to make cython optional at build time (it's nice to have but I prefer to keep the hard requirements as low as possible). So, for testing, it might be easier to have 2 virtualenvs, one with and another without cython support.

In your environment, do you confirm that tests fail when the cython version is in use ?

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