FEAT-#3303: Add __getitem__ for Resampler #3613
Merged
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 #3613 +/- ##
===========================================
- Coverage 85.47% 58.82% -26.65%
===========================================
Files 197 183 -14
Lines 16320 15661 -659
===========================================
- Hits 13949 9213 -4736
- Misses 2371 6448 +4077
Continue to review full report at Codecov.
|
|
|
|
|
This pull request introduces 1 alert when merging 78cb77d5cf5499b8aa1c2fc0ad367ca77e1711a9 into 7a81588 - view on LGTM.com new alerts:
|
Rubtsowa
force-pushed
the
fix_resampler
branch
from
78cb77d
to
6d37505
Compare
prutskov
reviewed
Nov 3, 2021
Rubtsowa
force-pushed
the
fix_resampler
branch
2 times, most recently
from
9b0f683
to
707129b
Compare
prutskov
reviewed
Nov 10, 2021
Rubtsowa
force-pushed
the
fix_resampler
branch
from
707129b
to
54792eb
Compare
prutskov
reviewed
Nov 11, 2021
|
@Rubtsowa, address existing comments please |
prutskov
reviewed
Nov 15, 2021
prutskov
reviewed
Nov 16, 2021
There was a problem hiding this comment.
Please, address the old comments. Also, you didn't fully changed the behavior requested in
Lines 3112 to 3113 in 6178a57
But I don't mind if moving to dict using will be done in a separate PR as resolution of new issue (in that case create issue, please)
prutskov
reviewed
Nov 18, 2021
|
@modin-project/modin-core please take a look |
prutskov
reviewed
Nov 22, 2021
|
I created Issue #3724 |
YarShev
reviewed
Nov 23, 2021
prutskov
previously approved these changes
Nov 23, 2021
YarShev
reviewed
Nov 23, 2021
| "tuple", | ||
| "series", | ||
| "index", | ||
| "duplicated_column", |
There was a problem hiding this comment.
Suggested change
| "duplicated_column", | |
| "duplicate_column", |
YarShev
previously approved these changes
Nov 23, 2021
vnlitvinov
reviewed
Nov 23, 2021
Signed-off-by: Maria Rubtsova <maria.rubtsova@intel.com>
Signed-off-by: Maria Rubtsova <maria.rubtsova@intel.com>
Signed-off-by: Maria Rubtsova <maria.rubtsova@intel.com>
Signed-off-by: Maria Rubtsova <maria.rubtsova@intel.com>
Signed-off-by: Maria Rubtsova <maria.rubtsova@intel.com>
Signed-off-by: Maria Rubtsova <maria.rubtsova@intel.com>
Signed-off-by: Maria Rubtsova <maria.rubtsova@intel.com>
…nal tests Signed-off-by: Alexey Prutskov <alexey.prutskov@intel.com>
Signed-off-by: Maria Rubtsova <maria.rubtsova@intel.com>
Signed-off-by: Maria Rubtsova <maria.rubtsova@intel.com>
Signed-off-by: Maria Rubtsova <maria.rubtsova@intel.com>
Signed-off-by: Maria Rubtsova <maria.rubtsova@intel.com>
Rubtsowa
force-pushed
the
fix_resampler
branch
from
b4beef5
to
947aa9e
Compare
|
@modin-project/modin-core CI is green |
|
@vnlitvinov what do you think, are we good to go here or you have more comments on the changes? |
vnlitvinov
approved these changes
Nov 24, 2021
Comment on lines
+3147
to
+3149
| subset = self._dataframe[key] | ||
| resampler = type(self)(subset, *self.resample_args) | ||
| return resampler |
There was a problem hiding this comment.
Suggested change
| subset = self._dataframe[key] | |
| resampler = type(self)(subset, *self.resample_args) | |
| return resampler | |
| return Resampler(self._dataframe[key], *self.resample_args) |
I don't see a point in doing 3 lines instead of one here, unless you expect this method to be inherited by some child class?.. I mean this type(self)(args) thing looks cool but somewhat hacky.
There was a problem hiding this comment.
We might want to have a child class (like we have Dataframe(BasePandasDataset)) that inherits the method so I think it is fine.
YarShev
approved these changes
Nov 25, 2021
devin-petersohn
pushed a commit
that referenced
this pull request
Dec 15, 2021
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: Maria Rubtsova maria.rubtsova@intel.com
What do these changes do?
flake8 modin/ asv_bench/benchmarks scripts/doc_checker.pyblack --check modin/ asv_bench/benchmarks scripts/doc_checker.pygit commit -sdocs/developer/architecture.rstis up-to-date