Skip to content

Conversation

@HyukjinKwon
Copy link
Member

@HyukjinKwon HyukjinKwon commented May 18, 2020

What changes were proposed in this pull request?

This PR is kind of a followup for SPARK-29641 and SPARK-28234. This PR proposes:

1.. Document the new pyspark.resource module introduced at 95aec09, in PySpark API docs.

2.. Move classes into fewer and simpler modules

Before:

pyspark
├── resource
│   ├── executorrequests.py
│   │   ├── class ExecutorResourceRequest 
│   │   └── class ExecutorResourceRequests
│   ├── taskrequests.py
│   │   ├── class TaskResourceRequest 
│   │   └── class TaskResourceRequests
│   ├── resourceprofilebuilder.py
│   │   └── class ResourceProfileBuilder
│   ├── resourceprofile.py
│   │   └── class ResourceProfile
└── resourceinformation.py
    └── class ResourceInformation

After:

pyspark
└── resource
    ├── requests.py
    │   ├── class ExecutorResourceRequest 
    │   ├── class ExecutorResourceRequests
    │   ├── class TaskResourceRequest 
    │   └── class TaskResourceRequests
    ├── profile.py
    │   ├── class ResourceProfileBuilder
    │   └── class ResourceProfile
    └── information.py
        └── class ResourceInformation

3.. Minor docstring fix e.g.:

-     @param name the name of the resource
-     @param addresses an array of strings describing the addresses of the resource
+     :param name: the name of the resource
+     :param addresses: an array of strings describing the addresses of the resource
+
+     .. versionadded:: 3.0.0

Why are the changes needed?

To document APIs, and move Python modules to fewer and simpler modules.

Does this PR introduce any user-facing change?

No, the changes are in unreleased branches.

How was this patch tested?

Manually tested via:

cd python
./run-tests --python-executables=python3 --modules=pyspark-core
./run-tests --python-executables=python3 --modules=pyspark-resource

@HyukjinKwon
Copy link
Member Author

cc @holdenk too - seems I can't add you as a reviewer.

:param name: the name of the resource
:param addresses: an array of strings describing the addresses of the resource

.. versionadded:: 3.0.0
Copy link
Member Author

@HyukjinKwon HyukjinKwon May 18, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will open a PR to backport for this change specifically for branch-3.0. This PR itself should go to master only.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please make a PR to branch-3.0 for this part.

@SparkQA

This comment has been minimized.

@HyukjinKwon HyukjinKwon removed the DOCS label May 18, 2020
@SparkQA
Copy link

SparkQA commented May 18, 2020

Test build #122798 has finished for PR 28569 at commit 6fa1ab4.

  • This patch passes all tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

@HyukjinKwon HyukjinKwon requested a review from viirya May 18, 2020 18:45
Copy link
Member

@BryanCutler BryanCutler left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1 on the new module layout and the doc changes LGTM

Copy link
Member

@viirya viirya left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good too. It looks clearer.

@SparkQA
Copy link

SparkQA commented May 19, 2020

Test build #122824 has finished for PR 28569 at commit 6beeca7.

  • This patch fails PySpark unit tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

@viirya
Copy link
Member

viirya commented May 19, 2020

retest this please

@HyukjinKwon HyukjinKwon removed the DOCS label May 19, 2020
@SparkQA
Copy link

SparkQA commented May 19, 2020

Test build #122825 has finished for PR 28569 at commit 6beeca7.

  • This patch passes all tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

@dongjoon-hyun dongjoon-hyun changed the title [SPARK-31748][CORE][PYTHON] Document resource module in PySpark doc and rename/move classes [SPARK-31748][PYTHON] Document resource module in PySpark doc and rename/move classes May 20, 2020
Copy link
Member

@dongjoon-hyun dongjoon-hyun left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1, LGTM. Merged to master. Thank you.

@HyukjinKwon
Copy link
Member Author

Thank you all!

HyukjinKwon added a commit that referenced this pull request May 20, 2020
…d rename/move classes

### What changes were proposed in this pull request?

This PR partially backports #28569.

1.. Rename

```
pyspark
└── resourceinformation.py
    └── class ResourceInformation
```

to

```
pyspark
└── resource.py
    └── class ResourceInformation
```

So, the `ResourceInformation` is consistently imported via `pyspark.resource.ResourceInformation`.

2.. Document the new `pyspark.resource` module

3.. Minor docstring fix e.g.:

```diff
-     param name the name of the resource
-     param addresses an array of strings describing the addresses of the resource
+     :param name: the name of the resource
+     :param addresses: an array of strings describing the addresses of the resource
+
+     .. versionadded:: 3.0.0
```

### Why are the changes needed?

To document APIs, and move Python modules to fewer and simpler modules.

### Does this PR introduce _any_ user-facing change?

No, the changes are in unreleased branches.

### How was this patch tested?

Manually tested via:

```bash
cd python
./run-tests --python-executables=python3 --modules=pyspark-core
```

Closes #28586 from HyukjinKwon/SPARK-31748.

Authored-by: HyukjinKwon <gurwls223@apache.org>
Signed-off-by: HyukjinKwon <gurwls223@apache.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants