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

Plans to support Python3? #26

Closed
rw opened this issue Nov 27, 2015 · 9 comments
Closed

Plans to support Python3? #26

rw opened this issue Nov 27, 2015 · 9 comments

Comments

@rw
Copy link

rw commented Nov 27, 2015

Are there any plans to support Python3?

@dlecocq
Copy link
Contributor

dlecocq commented Nov 28, 2015

Not that I'm aware of. Though of course, we welcome pull requests :-)

@codinguncut
Copy link

I have started to port it to python 3, and would be happy to PR, but am still having some issues with cython "str" vs. "unicode"...
It may be necessary to retrain the sklearn model re: sklearn versions, but that should be OK if given training data, code and instructions are complete.
Another option may be cpy2py, but I haven't quite gotten it to work yet...

@matt-peters
Copy link
Collaborator

Excellent! How far along are you? You may want to coordinate with @bdewilde as has also given a python 3 port some thought. Also, there is an open PR #41 that will likely include some API changes and other large changes that may cause some conflicts with your branch.

@rw
Copy link
Author

rw commented Apr 5, 2017

I think if it were easier to regenerate the model then you could have the model serialized for both 2 and 3. Then, you could just switch based on the version (I think :-) ).

@nehalecky
Copy link
Contributor

Hey all, jumping in the py3 compatibility discussion for dragnet with @bdewilde on his PR #41 (which includes progress towards python 3 compatibility). Thought it best if we collaborate on this effort and break apart distinct efforts for refactoring—a py 3 porting roadmap—that could be defined by project leads and key contributors so we can parallelize the effort. Can we work towards this?

A few questions:

  • are unit tests up to date for supporting the refactor?
  • @bdewilde and others have mentioned significant effort regarding the blocks.pyx cython module, can we get a little more color on the effort there?
  • @rw and @codinguncut seem to also have ideas regarding the porting effort, was there any collaboration regarding such with @bdewilde about this. Clearly would be great to have a common vision towards any API changes and general designs. ;)

Thanks all, excited about joining the effort here. 🎆

@matt-peters
Copy link
Collaborator

I'm 100% supportive of an effort to port to Python 3. Unfortunately I won't be able to help much other then code reviews and merging PRs. For a roadmap, I'd suggest we first get #41 merged, then someone should take a look at blocks.pyx and figure out the major hurdles. I leave it to interested parties (@nehalecky, @bdewilde, @rw, @codinguncut) to self organize around these.

Regarding some context on blocks.pyx -- this is the major bit of code that interacts with the string parsing in lxml at a low level (via lxml's cython API). Unfortunately this bit of code is pretty ugly and has some performance hacks that obfuscate some of the logic. Fortunately it does have pretty good test coverage now and with luck some of the dirtier bits that handle string encoding won't be necessary in the Python 3 version.

@nehalecky
Copy link
Contributor

Hey All, we finally got up our PR (#52) for the completed port to python 3 (lot's of heavy lifting in streamlining string parsing by @pakelley). Look forward to your feedback there!

@pakelley
Copy link
Collaborator

Should this be closed now that #52 has been merged?

@b4hand
Copy link
Contributor

b4hand commented Feb 14, 2018

Yup.

@b4hand b4hand closed this as completed Feb 14, 2018
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

7 participants