FIX-#1729: Fix incorrect result of Series.dt.components/freq/tz
#1730
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Codecov Report
@@ Coverage Diff @@
## master #1730 +/- ##
==========================================
+ Coverage 78.25% 85.82% +7.57%
==========================================
Files 77 77
Lines 8287 9280 +993
==========================================
+ Hits 6485 7965 +1480
+ Misses 1802 1315 -487
Continue to review full report at Codecov.
|
|
|
|
YarShev
reviewed
Jul 15, 2020
prutskov
force-pushed
the
prutskov/dt-components-fix
branch
3 times, most recently
from
8ec1cdd
to
c47b83b
Compare
prutskov
force-pushed
the
prutskov/dt-components-fix
branch
6 times, most recently
from
b8a960c
to
a0d31c9
Compare
|
@prutskov please add test for the failing case. |
|
@anmyachev Test for dt.components already exists in tests. This test will be passed after #1727 will merged and current branch will be rebased (it was tested locally using fixes from #1727). Is it needed to add test for |
|
Not for I mean this reproducer: if __name__ == "__main__":
import modin.pandas as pd
import pandas
import numpy as np
data = pd.to_timedelta(np.arange(5), unit="d")
md_series, pd_series = pd.Series(data), pandas.Series(data)
print("pandas:\n", pd_series.dt.components)
print("\nmodin:\n", md_series.dt.components)As I understand, it should fail on master. If so, we need to add test with this reproducer that will fail before your changes and will pass after your changes. About what existed test are you speaking? |
|
@anmyachev reproducer you mentioned (from #1729) is actually a test case from @prutskov you could add tests where besides checking your results for equality with pandas with |
Ok, I got it. |
prutskov
force-pushed
the
prutskov/dt-components-fix
branch
from
a0d31c9
to
bb41034
Compare
|
@dchigarev, @anmyachev Test was added. |
prutskov
force-pushed
the
prutskov/dt-components-fix
branch
from
bb41034
to
61adcda
Compare
prutskov
changed the title
Series.dt.components
prutskov
force-pushed
the
prutskov/dt-components-fix
branch
from
61adcda
to
d21230b
Compare
prutskov
force-pushed
the
prutskov/dt-components-fix
branch
from
bab4271
to
c9129d4
Compare
prutskov
changed the title
Series.dt.components Series.dt.components
prutskov
force-pushed
the
prutskov/dt-components-fix
branch
from
c9129d4
to
8a821aa
Compare
prutskov
changed the title
Series.dt.components Series.dt.components/freq/tz
|
@dchigarev, @devin-petersohn Additional fixes for validating |
prutskov
force-pushed
the
prutskov/dt-components-fix
branch
3 times, most recently
from
eea3c65
to
2658b27
Compare
dchigarev
reviewed
Jul 27, 2020
dchigarev
reviewed
Jul 27, 2020
prutskov
force-pushed
the
prutskov/dt-components-fix
branch
2 times, most recently
from
222c1ef
to
b0c0343
Compare
Data size was increased. Problem in |
dchigarev
reviewed
Jul 27, 2020
dchigarev
requested changes
Jul 27, 2020
prutskov
force-pushed
the
prutskov/dt-components-fix
branch
from
b0c0343
to
311dd3c
Compare
This was applied. |
Signed-off-by: Alexey Prutskov <alexey.prutskov@intel.com>
prutskov
force-pushed
the
prutskov/dt-components-fix
branch
from
311dd3c
to
aeb784a
Compare
dchigarev
approved these changes
Jul 27, 2020
There was a problem hiding this comment.
LGTM, assuming that it passes all tests.
@devin-petersohn ?
devin-petersohn
approved these changes
Jul 28, 2020
aregm
pushed a commit
to aregm/modin
that referenced
this pull request
Sep 16, 2020
…odin-project#1730) Signed-off-by: Alexey Prutskov <alexey.prutskov@intel.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Signed-off-by: Alexey Prutskov alexey.prutskov@intel.com
What do these changes do?
flake8 modinblack --check modingit commit -s