You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@beam.apache.org by GitBox <gi...@apache.org> on 2020/04/17 18:56:01 UTC
[GitHub] [beam] robertwb opened a new pull request #11449: Updates and
clarifications on type safety.
robertwb opened a new pull request #11449: Updates and clarifications on type safety.
URL: https://github.com/apache/beam/pull/11449
* Clarifiactions about when errors are raised.
* Clarifications about PickleCoder. (The default is actually FastPrimitivesCoder, but this is an implementation detail.)
* Users should not be calling ParDo with a function.
* Removes unused (and not reccomended) *args and **kwargs in process methods.
------------------------
Thank you for your contribution! Follow this checklist to help us incorporate your contribution quickly and easily:
- [ ] [**Choose reviewer(s)**](https://beam.apache.org/contribute/#make-your-change) and mention them in a comment (`R: @username`).
- [ ] Format the pull request title like `[BEAM-XXX] Fixes bug in ApproximateQuantiles`, where you replace `BEAM-XXX` with the appropriate JIRA issue, if applicable. This will automatically link the pull request to the issue.
- [ ] Update `CHANGES.md` with noteworthy changes.
- [ ] If this contribution is large, please file an Apache [Individual Contributor License Agreement](https://www.apache.org/licenses/icla.pdf).
See the [Contributor Guide](https://beam.apache.org/contribute) for more tips on [how to make review process smoother](https://beam.apache.org/contribute/#make-reviewers-job-easier).
Post-Commit Tests Status (on master branch)
------------------------------------------------------------------------------------------------
Lang | SDK | Apex | Dataflow | Flink | Gearpump | Samza | Spark
--- | --- | --- | --- | --- | --- | --- | ---
Go | [](https://builds.apache.org/job/beam_PostCommit_Go/lastCompletedBuild/) | --- | --- | [](https://builds.apache.org/job/beam_PostCommit_Go_VR_Flink/lastCompletedBuild/) | --- | --- | [](https://builds.apache.org/job/beam_PostCommit_Go_VR_Spark/lastCompletedBuild/)
Java | [](https://builds.apache.org/job/beam_PostCommit_Java/lastCompletedBuild/) | [](https://builds.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Apex/lastCompletedBuild/) | [](https://builds.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Dataflow/lastCompletedBuild/)<br>[](https://builds.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Dataflow_Java11/lastCompletedBuild/) | [](https://builds.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Flink/lastCompletedBuild/)<br>[](https://builds.apache.org/job/beam_PostCommit_Java_PVR_Flink_Batch/lastCompletedBuild/)<br>[](https://builds.apache.org/job/beam_PostCommit_Java_PVR_Flink_Streaming/lastCompletedBuild/) | [](https://builds.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Gearpump/lastCompletedBuild/) | [](https://builds.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Samza/lastCompletedBuild/) | [](https://builds.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Spark/lastCompletedBuild/)<br>[](https://builds.apache.org/job/beam_PostCommit_Java_PVR_Spark_Batch/lastCompletedBuild/)<br>[](https://builds.apache.org/job/beam_PostCommit_Java_ValidatesRunner_SparkStructuredStreaming/lastCompletedBuild/)
Python | [](https://builds.apache.org/job/beam_PostCommit_Python2/lastCompletedBuild/)<br>[](https://builds.apache.org/job/beam_PostCommit_Python35/lastCompletedBuild/)<br>[](https://builds.apache.org/job/beam_PostCommit_Python36/lastCompletedBuild/)<br>[](https://builds.apache.org/job/beam_PostCommit_Python37/lastCompletedBuild/) | --- | [](https://builds.apache.org/job/beam_PostCommit_Py_VR_Dataflow/lastCompletedBuild/)<br>[](https://builds.apache.org/job/beam_PostCommit_Py_VR_Dataflow_V2/lastCompletedBuild/)<br>[](https://builds.apache.org/job/beam_PostCommit_Py_ValCont/lastCompletedBuild/) | [](https://builds.apache.org/job/beam_PreCommit_Python2_PVR_Flink_Cron/lastCompletedBuild/)<br>[](https://builds.apache.org/job/beam_PostCommit_Python35_VR_Flink/lastCompletedBuild/) | --- | --- | [](https://builds.apache.org/job/beam_PostCommit_Python_VR_Spark/lastCompletedBuild/)
XLang | --- | --- | --- | [](https://builds.apache.org/job/beam_PostCommit_XVR_Flink/lastCompletedBuild/) | --- | --- | [](https://builds.apache.org/job/beam_PostCommit_XVR_Spark/lastCompletedBuild/)
Pre-Commit Tests Status (on master branch)
------------------------------------------------------------------------------------------------
--- |Java | Python | Go | Website
--- | --- | --- | --- | ---
Non-portable | [](https://builds.apache.org/job/beam_PreCommit_Java_Cron/lastCompletedBuild/) | [](https://builds.apache.org/job/beam_PreCommit_Python_Cron/lastCompletedBuild/)<br>[](https://builds.apache.org/job/beam_PreCommit_PythonLint_Cron/lastCompletedBuild/) | [](https://builds.apache.org/job/beam_PreCommit_Go_Cron/lastCompletedBuild/) | [](https://builds.apache.org/job/beam_PreCommit_Website_Cron/lastCompletedBuild/)
Portable | --- | [](https://builds.apache.org/job/beam_PreCommit_Portable_Python_Cron/lastCompletedBuild/) | --- | ---
See [.test-infra/jenkins/README](https://github.com/apache/beam/blob/master/.test-infra/jenkins/README.md) for trigger phrase, status and link of all Jenkins jobs.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [beam] udim commented on a change in pull request #11449: Updates
and clarifications on type safety.
Posted by GitBox <gi...@apache.org>.
udim commented on a change in pull request #11449: Updates and clarifications on type safety.
URL: https://github.com/apache/beam/pull/11449#discussion_r410464926
##########
File path: website/src/documentation/sdks/python-type-safety.md
##########
@@ -91,8 +92,8 @@ The following code declares an `int` input and a `str` output type hint on the `
```
The following code declares `int` input and output type hints on `filter_evens`, using annotations on `FilterEvensDoFn.process`.
-Since `process` returns a generator, the output type is annotated as `Iterable[int]` (`Generator[int, None, None]` would also work here).
-Beam will remove the outer iterable of the return type on the `DoFn.process` method and functions passed to `ParDo` and `FlatMap`.
Review comment:
If you're removing mentions of ParDo here, then the corresponding snippets should also be updated to use FlatMap instead of ParDo.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [beam] robertwb commented on issue #11449: Updates and
clarifications on type safety.
Posted by GitBox <gi...@apache.org>.
robertwb commented on issue #11449: Updates and clarifications on type safety.
URL: https://github.com/apache/beam/pull/11449#issuecomment-615492524
Thanks!
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [beam] robertwb commented on a change in pull request #11449:
Updates and clarifications on type safety.
Posted by GitBox <gi...@apache.org>.
robertwb commented on a change in pull request #11449: Updates and clarifications on type safety.
URL: https://github.com/apache/beam/pull/11449#discussion_r410500050
##########
File path: website/src/documentation/sdks/python-type-safety.md
##########
@@ -91,8 +92,8 @@ The following code declares an `int` input and a `str` output type hint on the `
```
The following code declares `int` input and output type hints on `filter_evens`, using annotations on `FilterEvensDoFn.process`.
-Since `process` returns a generator, the output type is annotated as `Iterable[int]` (`Generator[int, None, None]` would also work here).
-Beam will remove the outer iterable of the return type on the `DoFn.process` method and functions passed to `ParDo` and `FlatMap`.
Review comment:
The ParDos in the snippets are being passed DoFns. I removed the reference to passing (raw) functions to ParDo.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [beam] robertwb commented on a change in pull request #11449:
Updates and clarifications on type safety.
Posted by GitBox <gi...@apache.org>.
robertwb commented on a change in pull request #11449: Updates and clarifications on type safety.
URL: https://github.com/apache/beam/pull/11449#discussion_r410499350
##########
File path: sdks/python/apache_beam/examples/snippets/snippets_test_py3.py
##########
@@ -44,7 +44,7 @@ def test_bad_types_annotations(self):
# pylint: disable=expression-not-assigned
# pylint: disable=unused-variable
class FilterEvensDoFn(beam.DoFn):
- def process(self, element, *unused_args, **unused_kwargs):
Review comment:
Well, mypy and PyCharm are certainly in error here. Possibly it's because we call DoFns with *args and **kwargs in places, but we construct those to match the actual signature. Please file a JIRA about this.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [beam] robertwb merged pull request #11449: Updates and
clarifications on type safety.
Posted by GitBox <gi...@apache.org>.
robertwb merged pull request #11449: Updates and clarifications on type safety.
URL: https://github.com/apache/beam/pull/11449
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [beam] udim commented on a change in pull request #11449: Updates
and clarifications on type safety.
Posted by GitBox <gi...@apache.org>.
udim commented on a change in pull request #11449: Updates and clarifications on type safety.
URL: https://github.com/apache/beam/pull/11449#discussion_r410454232
##########
File path: website/src/documentation/sdks/python-type-safety.md
##########
@@ -91,8 +92,8 @@ The following code declares an `int` input and a `str` output type hint on the `
```
The following code declares `int` input and output type hints on `filter_evens`, using annotations on `FilterEvensDoFn.process`.
-Since `process` returns a generator, the output type is annotated as `Iterable[int]` (`Generator[int, None, None]` would also work here).
-Beam will remove the outer iterable of the return type on the `DoFn.process` method and functions passed to `ParDo` and `FlatMap`.
+Since `process` returns a generator, the output type for a DoFn producing a `PCollection[int]` is annotated as `Iterable[int]` (`Generator[int, None, None]` would also work here).
+Beam will remove the outer iterable of the return type on the `DoFn.process` method and functions passed `FlatMap` to deduce the element type of resulting PCollection .
Review comment:
```suggestion
Beam will remove the outer iterable of the return type on the `DoFn.process` method and functions passed to `FlatMap` to deduce the element type of resulting PCollection .
```
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [beam] udim commented on a change in pull request #11449: Updates
and clarifications on type safety.
Posted by GitBox <gi...@apache.org>.
udim commented on a change in pull request #11449: Updates and clarifications on type safety.
URL: https://github.com/apache/beam/pull/11449#discussion_r410453129
##########
File path: sdks/python/apache_beam/examples/snippets/snippets_test_py3.py
##########
@@ -44,7 +44,7 @@ def test_bad_types_annotations(self):
# pylint: disable=expression-not-assigned
# pylint: disable=unused-variable
class FilterEvensDoFn(beam.DoFn):
- def process(self, element, *unused_args, **unused_kwargs):
Review comment:
I put these `unused_` arguments because I ran this file through mypy and it was complaining about them not being there.
PyCharm has similar complaints. Any solution to this?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [beam] udim commented on a change in pull request #11449: Updates
and clarifications on type safety.
Posted by GitBox <gi...@apache.org>.
udim commented on a change in pull request #11449: Updates and clarifications on type safety.
URL: https://github.com/apache/beam/pull/11449#discussion_r410504389
##########
File path: sdks/python/apache_beam/examples/snippets/snippets_test_py3.py
##########
@@ -44,7 +44,7 @@ def test_bad_types_annotations(self):
# pylint: disable=expression-not-assigned
# pylint: disable=unused-variable
class FilterEvensDoFn(beam.DoFn):
- def process(self, element, *unused_args, **unused_kwargs):
Review comment:
I believe it's because the base class has *args and **kwargs in the prototype:
https://github.com/apache/beam/blob/1de50c348706ed25af2bab9c9477d7d4f36ef8bf/sdks/python/apache_beam/transforms/core.py#L621
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services