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

Patch which makes the module work in Python 3 #16

Closed
wants to merge 1 commit into from

Conversation

amacbee
Copy link

@amacbee amacbee commented Oct 9, 2014

Here is a patch which makes the module work in Python 3..
isinstance(basestring) raises a NameError in Python 3.
because type of basestring is not supported.

@xgdgsc
Copy link

xgdgsc commented Mar 16, 2015

Any dev looking at this? I also get the error NameError: name 'basestring' is not defined using python3

@ThomasP6t
Copy link
Contributor

This can be more easily solved by modifying setup.py (see my pull request); upon installation the correct version of the condition will be in the code.

@ghost
Copy link

ghost commented Apr 29, 2015

I tried to apply the proposed debug, but i'm ending up, getting the error:
TypeError: fit() missing 1 required positional argument: 'X'
in sklearn_pandas/init.py", line 134, in fit
(raised by sklearn/base.py", line 426, in fit_transform)
is there any other fix i can apply?

Edit: I'm a dummy who did not initialize the sklearn.preprocessing class.

@ThomasP6t
Copy link
Contributor

MS-Position, can you give us a working example of the code that generates this error?

@ghost
Copy link

ghost commented May 4, 2015

I actually can not (see above). The patch does indeed work.
Thank You for Your post.

@dukebody
Copy link
Collaborator

dukebody commented Aug 9, 2015

I think this PR is not needed anymore since the Py3K problem was resolved with #15

@calpaterson
Copy link
Collaborator

I'd like to find a better way to do it than mutating basestring, but it does seem that this problem is solved at least for now

@dukebody
Copy link
Collaborator

We can look into six: https://pypi.python.org/pypi/six

@ThomasP6t
Copy link
Contributor

I made a new pull request which implements this using six: #32

@ThomasP6t
Copy link
Contributor

And another that does the same thing without the need for six: #33

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

5 participants