Skip to content

DOC/PY3: Python 3 docs example compat for range, StringIO and datetime #6230

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

Merged
2 commits merged into from
Feb 4, 2014

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.

@@ -698,7 +698,7 @@ Experimental

nrows, ncols = 20000, 100
df1, df2, df3, df4 = [DataFrame(randn(nrows, ncols))
for _ in xrange(4)]
for _ in list(range(4))]
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
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants