Update the CONTRIBUTING document. #2

michaelreneer · 2018-12-19T17:48:20Z

Replaced the Contributor License Agreements section with the equivalent
section from TensorFlow.
Added information about license and new files.

* Replaced the Contributor License Agreements section with the equivalent section from TensorFlow. * Added information about license and new files.

PiperOrigin-RevId: 226492993

There are a few users to consider: 1. A developer on TFF wants source at HEAD. 2. A researcher using TFF wants pip package at release in 90% of scenarios and sometimes (ideally rarely) wants nightly, but never wants to build a pip package. 3. A developer making a framework using TFF wants pip release and nightly, but never wants to build a pip package. 4. A researcher upstreaming change to TFF basically has moved from #2 to #3. Fixes: #878 PiperOrigin-RevId: 321226038

There are a few users to consider: 1. A developer on TFF wants source at HEAD. 2. A researcher using TFF wants pip package at release in 90% of scenarios and sometimes (ideally rarely) wants nightly, but never wants to build a pip package. 3. A developer making a framework using TFF wants pip release and nightly, but never wants to build a pip package. 4. A researcher upstreaming change to TFF basically has moved from google-parfait#2 to google-parfait#3. Fixes: #878 PiperOrigin-RevId: 321226038

This revealed two Python typing-related issues: 1. Every subclass of `executor_base.Executor` was using a property on the corresponding subclass of `executor_value_base.ExecutorValue` without having access to that property. This was fixed by: 2. The methods on the subclasses of `federating_executor.FederatingStrategy` (`FederatedComposingStrategy` and `FederatedResolvingStrategy`) were annotated to return types specific to that subclass. This return type conflicted with the return types of the shared implementations defined in `executor_utils`. 3. The type annotation of `target_executors` parameter of `FederatedResolvingStrategy` was incorrect. * Enabled pytype in the following modules: * `federated_composing_strategy` * `federated_resolving_strategy` * `federating_executor` * `reference_resolving_executor` * `remote_executor` * `thread_delegating_executor` * `value_serialization` To fix issue #1: * Promoted the `reference` property from the subclasses of `executor_value_base.ExecutorValue` to `executor_value_base.ExecutorValue`. * Renamed usage of this API to be consistent. To fix issue #2: * Updated the methods on the subclasses of `federating_executor.FederatingStrategy` (`FederatedComposingStrategy` and `FederatedResolvingStrategy`) to accept and return `executor_value_base.ExecutorValue` types. Note: That this does make the return types more generic, but I believe this is the intended interface. An alternative fix could be to update the `executor_utils` function to take and return `TypeVar`'s (i.e. generic functions). However, I don't think this is the intended interface. To fix issue #3: * Updated the type annotation of the `target_executors` parameter to `dict[placements.PlacementLiteral, Union[list[executor_base.Executor], executor_base.Executor]]`. PiperOrigin-RevId: 502890245

…cutors` package. This revealed two Python typing-related issues: 1. Every subclass of `executor_base.Executor` was using a property on the corresponding subclass of `executor_value_base.ExecutorValue` without having access to that property. This was fixed by: 2. The methods on the subclasses of `federating_executor.FederatingStrategy` (`FederatedComposingStrategy` and `FederatedResolvingStrategy`) were annotated to return types specific to that subclass. This return type conflicted with the return types of the shared implementations defined in `executor_utils`. 3. The type annotation of `target_executors` parameter of `FederatedResolvingStrategy` was incorrect. * Enabled pytype in the following modules: * `federated_composing_strategy` * `federated_resolving_strategy` * `federating_executor` * `reference_resolving_executor` * `remote_executor` * `thread_delegating_executor` * `value_serialization` To fix issue #1: * Promoted the `reference` property from the subclasses of `executor_value_base.ExecutorValue` to `executor_value_base.ExecutorValue`. * Renamed usage of this API to be consistent. To fix issue #2: * Updated the methods on the subclasses of `federating_executor.FederatingStrategy` (`FederatedComposingStrategy` and `FederatedResolvingStrategy`) to accept and return `executor_value_base.ExecutorValue` types. Note: That this does make the return types more generic, but I believe this is the intended interface. An alternative fix could be to update the `executor_utils` function to take and return `TypeVar`'s (i.e. generic functions). However, I don't think this is the intended interface. To fix issue #3: * Updated the type annotation of the `target_executors` parameter to `dict[placements.PlacementLiteral, Union[list[executor_base.Executor], executor_base.Executor]]`. PiperOrigin-RevId: 502890245

…cutors` package. This revealed two Python typing-related issues: 1. Every subclass of `executor_base.Executor` was using a property on the corresponding subclass of `executor_value_base.ExecutorValue` without having access to that property. This was fixed by: 2. The methods on the subclasses of `federating_executor.FederatingStrategy` (`FederatedComposingStrategy` and `FederatedResolvingStrategy`) were annotated to return types specific to that subclass. This return type conflicted with the return types of the shared implementations defined in `executor_utils`. 3. The type annotation of `target_executors` parameter of `FederatedResolvingStrategy` was incorrect. * Enabled pytype in the following modules: * `federated_composing_strategy` * `federated_resolving_strategy` * `federating_executor` * `reference_resolving_executor` * `remote_executor` * `thread_delegating_executor` * `value_serialization` To fix issue #1: * Promoted the `reference` property from the subclasses of `executor_value_base.ExecutorValue` to `executor_value_base.ExecutorValue`. * Renamed usage of this API to be consistent. To fix issue #2: * Updated the methods on the subclasses of `federating_executor.FederatingStrategy` (`FederatedComposingStrategy` and `FederatedResolvingStrategy`) to accept and return `executor_value_base.ExecutorValue` types. Note: That this does make the return types more generic, but I believe this is the intended interface. An alternative fix could be to update the `executor_utils` function to take and return `TypeVar`'s (i.e. generic functions). However, I don't think this is the intended interface. To fix issue #3: * Updated the type annotation of the `target_executors` parameter to `dict[placements.PlacementLiteral, Union[list[executor_base.Executor], executor_base.Executor]]`. PiperOrigin-RevId: 504114859

``` # expansion is pretty cool # https://www.gnu.org/software/bash/manual/html_node/Shell-Expansions.html echo "hello `world`" echo "hello ${world}" # single quotes disable expansion # https://www.gnu.org/software/bash/manual/html_node/Single-Quotes.html echo 'hello `world`' echo 'hello ${world}' # Problem - single quotes in single quotes echo 'hello ' world' # Fix #1: quote the single quote, which is exciting echo 'hello '"'"' world' # Fix #2: heredoc, maybe more readable but awkward # https://www.gnu.org/savannah-checkouts/gnu/bash/manual/bash.html#Here-Documents echo <<'EOF' hello ' world EOF ``` PiperOrigin-RevId: 675226634

``` # expansion is pretty cool # https://www.gnu.org/software/bash/manual/html_node/Shell-Expansions.html echo "hello `world`" echo "hello ${world}" # single quotes disable expansion # https://www.gnu.org/software/bash/manual/html_node/Single-Quotes.html echo 'hello `world`' echo 'hello ${world}' # Problem - single quotes in single quotes echo 'hello ' world' # Fix #1: quote the single quote, which is exciting echo 'hello '"'"' world' # Fix #2: heredoc, maybe more readable but awkward # https://www.gnu.org/savannah-checkouts/gnu/bash/manual/bash.html#Here-Documents echo <<'EOF' hello ' world EOF ``` PiperOrigin-RevId: 675586161

Update the CONTRIBUTING document.

1cd9df4

* Replaced the Contributor License Agreements section with the equivalent section from TensorFlow. * Added information about license and new files.

michaelreneer requested a review from ZacharyGarrett December 19, 2018 17:48

ZacharyGarrett approved these changes Dec 19, 2018

View reviewed changes

michaelreneer added enhancement New feature or request cla: yes and removed enhancement New feature or request labels Dec 19, 2018

michaelreneer requested a review from krzys-ostrowski December 19, 2018 22:56

michaelreneer removed the cla: yes label Dec 19, 2018

googlebot added the cla: yes label Dec 19, 2018

michaelreneer removed the request for review from krzys-ostrowski December 19, 2018 23:10

ZacharyGarrett approved these changes Dec 20, 2018

View reviewed changes

michaelreneer added enhancement New feature or request and removed enhancement New feature or request labels Dec 21, 2018

michaelreneer force-pushed the feature-docs branch from b7e2050 to 1cd9df4 Compare December 21, 2018 16:14

michaelreneer added enhancement New feature or request and removed enhancement New feature or request labels Dec 21, 2018

tensorflow-copybara merged commit 1cd9df4 into master Dec 21, 2018

tensorflow-copybara pushed a commit that referenced this pull request Dec 21, 2018

Merge pull request #2 from tensorflow:feature-docs

7620dbb

PiperOrigin-RevId: 226492993

michaelreneer deleted the feature-docs branch December 21, 2018 16:27

copybara-service bot mentioned this pull request Jan 18, 2023

Enabled and fixed python type annotations in some modules in the executors package. #3525

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Update the CONTRIBUTING document. #2

Update the CONTRIBUTING document. #2

michaelreneer commented Dec 19, 2018

Update the CONTRIBUTING document. #2

Update the CONTRIBUTING document. #2

Conversation

michaelreneer commented Dec 19, 2018