Remove implicit switch-ons of "compute.ops_on_diff_frames" #1953
Conversation
|
Shall we modify the docstrings of each touched function to reflect this change? |
xinrong-meng
changed the title
Codecov Report
@@ Coverage Diff @@
## master #1953 +/- ##
==========================================
- Coverage 94.64% 94.64% -0.01%
==========================================
Files 49 49
Lines 10822 10828 +6
==========================================
+ Hits 10243 10248 +5
- Misses 579 580 +1
Continue to review full report at Codecov.
|
|
I agree, too!
For this, IMO, I think we don't necessarily need to modify the docstrings since it already shows a proper error message - |
| combined = combine_frames(self.to_frame(), other.to_frame()) | ||
| if not self.index.equals(other.index): | ||
| raise ValueError("Can only compare identically-labeled Series objects") | ||
| combined = combine_frames(self.to_frame(), other.to_frame()) |
There was a problem hiding this comment.
Btw, we should consider the case where other is from the same anchor:
>>> s1 = ks.Series(["a", "b", "c", "d", "e"])
>>> s1.compare(s1)
Traceback (most recent call last):
...
AssertionError: We don't need to combine. `this` and `that` are same.Of course, this fix should be done in a separate PR.
There was a problem hiding this comment.
Good idea! Let me file a new PR for this.
Proposal