You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@beam.apache.org by "caneff (via GitHub)" <gi...@apache.org> on 2023/10/04 11:29:42 UTC
[PR] Add map function and skip applymap for 2.1 [beam]
caneff opened a new pull request, #28818:
URL: https://github.com/apache/beam/pull/28818
In Pandas 2.1, applymap is deprecated and map is the replacement. Fix it here for those
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add map function and skip applymap for 2.1 [beam]
Posted by "caneff (via GitHub)" <gi...@apache.org>.
caneff commented on PR #28818:
URL: https://github.com/apache/beam/pull/28818#issuecomment-1746684995
R: @tvalentyn
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add map function and skip applymap for 2.1 [beam]
Posted by "tvalentyn (via GitHub)" <gi...@apache.org>.
tvalentyn commented on code in PR #28818:
URL: https://github.com/apache/beam/pull/28818#discussion_r1346356281
##########
sdks/python/apache_beam/dataframe/frames_test.py:
##########
@@ -865,6 +865,7 @@ def test_corrwith_bad_axis(self):
self._run_error_test(lambda df: df.corrwith(df, axis=5), df)
@unittest.skipIf(PD_VERSION < (1, 2), "na_action added in pandas 1.2.0")
+ @unittest.skipIf(PD_VERSION >= (2, 1), "applymap deprecated in 2.1.0")
Review Comment:
is there harm to keep testing applymap until we remove the functionality?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add map function and skip applymap for 2.1 [beam]
Posted by "codecov[bot] (via GitHub)" <gi...@apache.org>.
codecov[bot] commented on PR #28818:
URL: https://github.com/apache/beam/pull/28818#issuecomment-1746766974
## [Codecov](https://app.codecov.io/gh/apache/beam/pull/28818?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
> Merging [#28818](https://app.codecov.io/gh/apache/beam/pull/28818?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) (ebbb8eb) into [master](https://app.codecov.io/gh/apache/beam/commit/12b071402035a0404c4f6e4a83124b3839b5de40?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) (12b0714) will **increase** coverage by `0.01%`.
> Report is 3 commits behind head on master.
> The diff coverage is `50.00%`.
```diff
@@ Coverage Diff @@
## master #28818 +/- ##
==========================================
+ Coverage 72.23% 72.24% +0.01%
==========================================
Files 684 684
Lines 101230 101232 +2
==========================================
+ Hits 73120 73135 +15
+ Misses 26532 26519 -13
Partials 1578 1578
```
| [Flag](https://app.codecov.io/gh/apache/beam/pull/28818/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Coverage Δ | |
|---|---|---|
| [python](https://app.codecov.io/gh/apache/beam/pull/28818/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `82.78% <50.00%> (+0.02%)` | :arrow_up: |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#carryforward-flags-in-the-pull-request-comment) to find out more.
| [Files](https://app.codecov.io/gh/apache/beam/pull/28818?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Coverage Δ | |
|---|---|---|
| [sdks/python/apache\_beam/dataframe/frames.py](https://app.codecov.io/gh/apache/beam/pull/28818?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vZGF0YWZyYW1lL2ZyYW1lcy5weQ==) | `95.16% <50.00%> (-0.04%)` | :arrow_down: |
... and [7 files with indirect coverage changes](https://app.codecov.io/gh/apache/beam/pull/28818/indirect-changes?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)
:mega: We’re building smart automated test selection to slash your CI/CD build times. [Learn more](https://about.codecov.io/iterative-testing/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add map function and skip applymap for 2.1 [beam]
Posted by "caneff (via GitHub)" <gi...@apache.org>.
caneff commented on code in PR #28818:
URL: https://github.com/apache/beam/pull/28818#discussion_r1346373400
##########
sdks/python/apache_beam/dataframe/frames_test.py:
##########
@@ -865,6 +865,7 @@ def test_corrwith_bad_axis(self):
self._run_error_test(lambda df: df.corrwith(df, axis=5), df)
@unittest.skipIf(PD_VERSION < (1, 2), "na_action added in pandas 1.2.0")
+ @unittest.skipIf(PD_VERSION >= (2, 1), "applymap deprecated in 2.1.0")
Review Comment:
Suppressed the warning instead.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add map function and skip applymap for 2.1 [beam]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #28818:
URL: https://github.com/apache/beam/pull/28818#issuecomment-1746688997
Stopping reviewer notifications for this pull request: review requested by someone other than the bot, ceding control
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add map function and skip applymap for 2.1 [beam]
Posted by "tvalentyn (via GitHub)" <gi...@apache.org>.
tvalentyn commented on PR #28818:
URL: https://github.com/apache/beam/pull/28818#issuecomment-1747527816
can merge once tests pass
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add map function and skip applymap for 2.1 [beam]
Posted by "tvalentyn (via GitHub)" <gi...@apache.org>.
tvalentyn commented on code in PR #28818:
URL: https://github.com/apache/beam/pull/28818#discussion_r1346361320
##########
sdks/python/apache_beam/dataframe/frames_test.py:
##########
@@ -865,6 +865,7 @@ def test_corrwith_bad_axis(self):
self._run_error_test(lambda df: df.corrwith(df, axis=5), df)
@unittest.skipIf(PD_VERSION < (1, 2), "na_action added in pandas 1.2.0")
+ @unittest.skipIf(PD_VERSION >= (2, 1), "applymap deprecated in 2.1.0")
Review Comment:
i would keep the test in this case if we care about it, unless we remove support from Beam SDK for it.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add map function and skip applymap for 2.1 [beam]
Posted by "tvalentyn (via GitHub)" <gi...@apache.org>.
tvalentyn commented on code in PR #28818:
URL: https://github.com/apache/beam/pull/28818#discussion_r1346145020
##########
sdks/python/apache_beam/dataframe/frames.py:
##########
@@ -2978,6 +2978,8 @@ def aggregate(self, func, axis, *args, **kwargs):
agg = aggregate
applymap = frame_base._elementwise_method('applymap', base=pd.DataFrame)
Review Comment:
do we need to skip applymap for 2.1 around line 2980?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add map function and skip applymap for 2.1 [beam]
Posted by "tvalentyn (via GitHub)" <gi...@apache.org>.
tvalentyn merged PR #28818:
URL: https://github.com/apache/beam/pull/28818
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add map function and skip applymap for 2.1 [beam]
Posted by "caneff (via GitHub)" <gi...@apache.org>.
caneff commented on code in PR #28818:
URL: https://github.com/apache/beam/pull/28818#discussion_r1346359731
##########
sdks/python/apache_beam/dataframe/frames_test.py:
##########
@@ -865,6 +865,7 @@ def test_corrwith_bad_axis(self):
self._run_error_test(lambda df: df.corrwith(df, axis=5), df)
@unittest.skipIf(PD_VERSION < (1, 2), "na_action added in pandas 1.2.0")
+ @unittest.skipIf(PD_VERSION >= (2, 1), "applymap deprecated in 2.1.0")
Review Comment:
I'm just trying to clean up deprecation warnings. I could suppress that warning instead.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add map function and skip applymap for 2.1 [beam]
Posted by "caneff (via GitHub)" <gi...@apache.org>.
caneff commented on code in PR #28818:
URL: https://github.com/apache/beam/pull/28818#discussion_r1346303626
##########
sdks/python/apache_beam/dataframe/frames.py:
##########
@@ -2978,6 +2978,8 @@ def aggregate(self, func, axis, *args, **kwargs):
agg = aggregate
applymap = frame_base._elementwise_method('applymap', base=pd.DataFrame)
Review Comment:
So the thing is that it isn't removed in 2.1, it is just deprecated. Also, the replacement, `map`, isn't in Pandas until 2.1 so there isn't time to change over otherwise. We still want to include it until it is removed.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org