Skip to content

Conversation

@phaebz
Copy link
Contributor

@phaebz phaebz commented Feb 2, 2014

related is #6212

Fixes examples in documentation to be compatible with Python 3. Also tested it locally under Python 2.

  • Turned occurrences of (x)range() to range() for iteration and to list(range()) for list concat.
  • Fixed leading zero argument(s) as in e.g. datetime(2013, 01, 27).
  • Removed explicit StringIO and cStringIO imports in examples (uses suppressed pandas.compat imports) and added a note on py2/3 library reorganization.
  • Fixed StringIO for binary data to the more explicit BytesIO.
  • Fixed NameError for missing pd. in pd.Series.
  • Added note for failing numpy.unique example, see also DOC/PY3: np.unique errors on pd.Series with mixed string/float in python3 #6229.

I am not sure about the BytesIO though. Maybe good to put a note saying this is Python 3 way to do it and users on 2.x should use the old StringIO instead. In general, should we favor Python 3 doc versions in such cases and add notes for Python 2.x users? What do you think?

@jreback
Copy link
Contributor

jreback commented Feb 2, 2014

seems ok to me...@y-p ?

@jorisvandenbossche
Copy link
Member

Is the conversion of xrange() to list(range(..)) necessary? Why not just to range()?
This works both on both python 2/3 in a list comprehension, and seems easier.

@ghost
Copy link

ghost commented Feb 2, 2014

as long as the pandas.compat imports are limited to wrappers for known classes
it's not a problem. Looks fine.

Wherever the code does not explicitly require a sequence for random access e.g. foo[4],
the list(range(foo)) is not required as joris pointed out.

@jreback
Copy link
Contributor

jreback commented Feb 2, 2014

yep you can just use range (if u r iterating over it)

use list(range) if you actually need a list

@phaebz
Copy link
Contributor Author

phaebz commented Feb 2, 2014

Adapted.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

forgot this one I think?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for pushing my nose into it... amended.

@phaebz
Copy link
Contributor Author

phaebz commented Feb 3, 2014

If this looks ok to you now, I could go ahead and squash...

@jreback
Copy link
Contributor

jreback commented Feb 3, 2014

go ahead and squash

@phaebz
Copy link
Contributor Author

phaebz commented Feb 3, 2014

done.

@ghost
Copy link

ghost commented Feb 4, 2014

@phaebz , please get rid of all the `list(range())`` where not required, I think that's
everywhere.

@jorisvandenbossche
Copy link
Member

@y-p regarding the PR, I think it is OK now. The remaining list in list(range(1,24))+[np.NAN] is needed for being able to add the nan.
But elsewhere in the docs there will be still some unneeded list(range())s, but that can maybe be left for another PR.

@ghost
Copy link

ghost commented Feb 4, 2014

You're right, good. merging.

ghost pushed a commit that referenced this pull request Feb 4, 2014
DOC/PY3: Python 3 docs example compat for range, StringIO and datetime
@ghost ghost merged commit 554c717 into pandas-dev:master Feb 4, 2014
This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

3 participants