-
Notifications
You must be signed in to change notification settings - Fork 358
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
Fix first/last_valid_index to support empty column DataFrame. #1923
Conversation
Codecov Report
@@ Coverage Diff @@
## master #1923 +/- ##
==========================================
- Coverage 94.15% 94.14% -0.01%
==========================================
Files 41 41
Lines 10007 10011 +4
==========================================
+ Hits 9422 9425 +3
- Misses 585 586 +1
Continue to review full report at Codecov.
|
@@ -1687,7 +1688,7 @@ def bool(self) -> bool: | |||
raise TypeError("bool() expects DataFrame or Series; however, " "got [%s]" % (self,)) | |||
return df.head(2)._to_internal_pandas().bool() | |||
|
|||
def first_valid_index(self) -> Union[Any, Tuple[Any, ...]]: | |||
def first_valid_index(self) -> Optional[Union[Any, Tuple[Any, ...]]]: | |||
""" | |||
Retrieves the index of the first valid value. | |||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Shall we modify Returns
in docstrings by including None
?
|
||
def last_valid_index(self) -> Union[Any, Tuple[Any, ...]]: | ||
def last_valid_index(self) -> Optional[Union[Any, Tuple[Any, ...]]]: | ||
""" | ||
Return index for last non-NA/null value. | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ditto.
@@ -1863,25 +1868,32 @@ def last_valid_index(self) -> Union[Any, Tuple[Any, ...]]: | |||
>>> s.last_valid_index() # doctest: +SKIP | |||
('cow', 'weight') | |||
""" | |||
sdf = self._internal.spark_frame | |||
if LooseVersion(pyspark.__version__) < LooseVersion("3.0"): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's better than specifying in docstrings only!
Other than two docstring improvements, LGTM! |
Thanks! merging. |
Fixes
first/last_valid_index
to support empty column DataFrame.E.g.,:
, which should return
None
:This PR also includes: