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

Add Python 3 support (and drop support for < 2.6) #9

Closed
wants to merge 10 commits into from
Closed

Add Python 3 support (and drop support for < 2.6) #9

wants to merge 10 commits into from

Conversation

dan-blanchard
Copy link
Member

Most of the credit for this goes to @bsidhom. I just took his Python 3 port and added a bunch of __future__ imports and the occasional from io import open to make it backward compatible with 2.6 and 2.7.

I did some minor clean-up things like sorting imports and things like that. Oh and I added a .gitattributes file to ensure line endings are consistent.

@erikrose Are you still actively maintaining this? I notice there a few outstanding pull requests and I just want to make sure the version on PyPI is 2/3 compatible soon.

@erikrose
Copy link
Contributor

I'm not really; I was kind of a drive-by patcher myself, but I saw no one else had a DVCS repo for it, and I managed to finagle maintainer privs on the PyPI entry, which hadn't been updated in forever.

You seem like you care about the project. Would you like to maintain it?

In fact, you've been drafted. ;-) I gave you commit privs on my repo. Do what you like, say "go", and I'll cut a release.

Cheers!
Erik

On Dec 10, 2013, at 3:39 PM, Dan Blanchard notifications@github.com wrote:

@erikrose Are you still actively maintaining this? I notice there a few outstanding pull requests and I just want to make sure the version on PyPI is 2/3 compatible soon.

@dan-blanchard
Copy link
Member Author

Thanks! I'll see if I can get Travis-CI setup with the unit tests from #4 and then we can automatically check if all the PRs actually work. 😄

@erikrose
Copy link
Contributor

Good first step!

@bsidhom
Copy link

bsidhom commented Dec 11, 2013

I actually discovered a while after making that fork that there was an effort to maintain a 2-and-3-compatible version. It's called charade: https://github.com/sigmavirus24/charade. I'm not sure if it's kept in sync with chardet though (or whether chardet itself is being maintained for that matter). You may want to reach out to @sigmavirus24 about it.

@dan-blanchard
Copy link
Member Author

Interesting... maybe we should just redirect people to using that then? I mean, if requests uses it, it's probably pretty sturdy, right?

@sigmavirus24
Copy link
Member

I have been doing an awful job of keeping it up to date with anything. We have 26 failing tests that were failing when I forked the repo. Pull requests are extraordinarily welcome and if anyone would like to help maintain it, please let me know. I'm kind of overwhelmed since I've started planning out how to bring PyYAML up-to-date with the current day.

@dan-blanchard
Copy link
Member Author

charade looks to be exactly what I was hoping we could do with this PR (and chardet in general), so maybe a merger makes the most sense.

Since more people know the name chardet (I think), @sigmavirus24, would you be willing to change the name back and create a PR into this repo? I don't want to step any toes, I just feel like consolidating our efforts would be better for everyone (and remove some of the confusion people have about the state of chardet).

@sigmavirus24
Copy link
Member

One word of warning @dan-blanchard: charade currently only supports 2.6, 2.7, 3.2, and 3.3 because that's all requests needs. If we merge, the changes which make it work on those might break compatibility with 2.5 if anyone cares about that. Does anyone have any opinions on this before I submit the PR?

@sigmavirus24
Copy link
Member

Also, I removed the necessity to install the script to chardetect.py or whatever it was called (the important thing being removing the .py from the end)

@dan-blanchard
Copy link
Member Author

I have no love for 2.5, and this PR already drops support for it, so it's fine by me. IMHO, it's time for people to move on to Python 3.

@sigmavirus24
Copy link
Member

I'll get around to this tomorrow most likely. If you would rather, feel free to create the PR yourself, since you should be able to select that as the HEAD for comparison. I'd rather use a separate branch so PR review can be addressed there.

@dan-blanchard
Copy link
Member Author

I'd rather use a separate branch so PR review can be addressed there.

@sigmavirus24, you mean you'll create a separate branch on the charade repo and create a PR from that branch into the master branch here?

Speaking of branches, this may be a conversation for another day, but I think switching to gitflow for our workflow would be nice. It's what I use on all my other repos. Actually, I just looked and it appears that's what you're using for charade (at least, you have separate develop and master branches which make me think so). So since @erikrose is pretty flexible about the direction we take things, I'm going to assume he won't mind that switch. 😄

I'll create a develop branch for you to select as the HEAD for comparison (it'll have the same content as the current master).

@sigmavirus24
Copy link
Member

That's exactly what I mean. I'm on my way into work and I have plans at lunch, but I should be able to take care of this tonight.

@dan-blanchard
Copy link
Member Author

Okay, cool. The develop branch is there now for you to use as the HEAD.

@erikrose
Copy link
Contributor

You guys rock; I'm enjoying watching the show. @dan-blanchard, let me know if anybody else needs privs. If this really takes off, we can always move it to a team-owned account like nose and pypa do to save management bottleneck.

@sigmavirus24
Copy link
Member

Frankly I've been trying to find someone interested in taking over charade for me for quite some time now. I'll help administer the repository and get debugging information but frankly my interest in maintaining the library as a whole is not very high. That said, I don't see at the moment that y'all have set up Travis with this repository. That's something I had set up with charade and for which you'll just need to flip a switch to enable. Let me know if you run into any issues with that and if you'd still like me to stick around.

@dan-blanchard
Copy link
Member Author

@sigmavirus24, I've only very recently joined the chardet team, and getting Travis setup was very high on my to-do list, so I'm glad that you've already got that setup.

Anyway, it's true that for all of use chardet/charade is not a huge priority, so I'm not sure how much we can expect to see development pick up even after this merger, but at least there will be more people begrudgingly getting the things done that need to get done. 😄

It's also possible that afterftfy 3.1 (rspeer/python-ftfy#16) comes out (with it's own encoding detection) that more and more of chardet's functionality will be superseded by ftfy. It's always nice to have options though.

dan-blanchard pushed a commit that referenced this pull request Dec 15, 2013
@dan-blanchard
Copy link
Member Author

Closing this, since it was superseded by #10.

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.

None yet

4 participants