Skip to content

Minor fix to SPARK-6958: Improve Python docstring for DataFrame.sort. #5558

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.

Sign up for GitHub

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

Closed
rxin wants to merge 2 commits into from
Closed

Minor fix to SPARK-6958: Improve Python docstring for DataFrame.sort. #5558

Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
d2dcf24
Minor fix to SPARK-6958: Improve Python docstring for DataFrame.sort.
rxin Apr 17, 2015
f4c276f
Review feedback.
rxin Apr 17, 2015
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 4 additions & 3 deletions python/pyspark/sql/dataframe.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -489,8 +489,9 @@ def sort(self, *cols, **kwargs):
"""Returns a new :class:`DataFrame` sorted by the specified column(s).

:param cols: list of :class:`Column` or column names to sort by.
:param ascending: sort by ascending order or not, could be bool, int
or list of bool, int (default: True).
:param ascending: boolean or list of boolean/int (default True).
Sort ascending vs. descending. Specify list for multiple sort orders.
If a list is specified, length of the list must equal length of the `cols`.

>>> df.sort(df.age.desc()).collect()
[Row(age=5, name=u'Bob'), Row(age=2, name=u'Alice')]
Expand Down Expand Up @@ -519,7 +520,7 @@ def sort(self, *cols, **kwargs):
jcols = [jc if asc else jc.desc()
for asc, jc in zip(ascending, jcols)]
else:
raise TypeError("ascending can only be bool or list, but got %s" % type(ascending))
raise TypeError("ascending can only be boolean or list, but got %s" % type(ascending))
Copy link
Contributor

@davies davies Apr 17, 2015

Choose a reason for hiding this comment

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

In Python, it's called bool

Copy link
Contributor Author

@rxin rxin Apr 17, 2015

Choose a reason for hiding this comment

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

I think bool is the name of the function, but the data type is still boolean?

see

http://pandas.pydata.org/pandas-docs/stable/generated/pandas.DataFrame.sort.html#pandas.DataFrame.sort

https://docs.python.org/2/library/stdtypes.html#boolean-values

https://docs.python.org/2/c-api/bool.html

Copy link
Contributor

@davies davies Apr 17, 2015

Choose a reason for hiding this comment

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

The type for boolean is bool:

>>> type(True)
<type 'bool'>
>>> type(1)
<type 'int'>

Similar to int for integer.

Copy link
Contributor

@davies davies Apr 17, 2015

Choose a reason for hiding this comment

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

It's weird to see we put boolean and int together, it should be either boolean/integer or bool/int.

Copy link
Contributor Author

@rxin rxin Apr 17, 2015

Choose a reason for hiding this comment

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

Alright I removed int, since 1, 0 has the same value as True, False anyway.

Copy link
Contributor

@davies davies Apr 17, 2015

Choose a reason for hiding this comment

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

Could you also change the tests in doc string to use True and False?

Copy link
Contributor Author

@rxin rxin Apr 17, 2015

Choose a reason for hiding this comment

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

then that's different from the Pandas examples?

Copy link
Contributor

@davies davies Apr 17, 2015

Choose a reason for hiding this comment

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

yeah, no perfect.


jdf = self._jdf.sort(self._jseq(jcols))
return DataFrame(jdf, self.sql_ctx)
Expand Down