Skip to content

[SPARK-26822]Upgrade the deprecated module 'optparse' #23730

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Closed
cchung100m wants to merge 5 commits into from
Closed

[SPARK-26822]Upgrade the deprecated module 'optparse' #23730

cchung100m wants to merge 5 commits into from

Conversation

@cchung100m
Copy link

@cchung100m cchung100m commented Feb 3, 2019 •
edited
Loading

Follow the official document to upgrade the deprecated module 'optparse' to 'argparse'.

What changes were proposed in this pull request?

This PR proposes to replace 'optparse' module with 'argparse' module.

How was this patch tested?

Follow the previous testing, manually tested and negative tests were also done. My test results

@cchung100m cchung100m changed the title Upgrade the deprecated module 'optparse' [SPARK-26822]Upgrade the deprecated module 'optparse' Feb 3, 2019
@HyukjinKwon
Copy link
Member

HyukjinKwon commented Feb 3, 2019

Yea I noticed it before.

@HyukjinKwon
Copy link
Member

HyukjinKwon commented Feb 3, 2019

ok to test

@HyukjinKwon
Copy link
Member

HyukjinKwon commented Feb 3, 2019

Can you keep the PR template? How did you test?

@SparkQA
Copy link

SparkQA commented Feb 3, 2019

Test build #102024 has finished for PR 23730 at commit 469fa31.

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

@felixcheung
Copy link
Member

felixcheung commented Feb 4, 2019 

Traceback (most recent call last):
  File "./python/run-tests.py", line 319, in <module>
    main()
  File "./python/run-tests.py", line 227, in main
    opts = parse_opts()
  File "./python/run-tests.py", line 176, in parse_opts
    help="A comma-separated list of Python executables to test against (default: %default)"
  File "/home/anaconda/envs/py3k/lib/python3.4/argparse.py", line 1340, in add_argument
    raise ValueError('%r is not callable' % (type_func,))
ValueError: 'string' is not callable

@cchung100m
Copy link
Author

cchung100m commented Feb 4, 2019

@HyukjinKwon , @felixcheung

Thanks for the prompt reply and suggestions, I will commit the update ASAP.

srowen
srowen requested changes Feb 4, 2019
View reviewed changes
Copy link
Member

@srowen srowen 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 pending tests but dev/run-tests.py needs a similar change.

@cchung100m cchung100m force-pushed the solve_deprecated_module_optparse branch from 469fa31 to 6e7ba67 Compare February 9, 2019 02:31
@SparkQA
Copy link

SparkQA commented Feb 9, 2019

Test build #102111 has finished for PR 23730 at commit 6e7ba67.

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

@cchung100m
Copy link
Author

cchung100m commented Feb 9, 2019

@HyukjinKwon , @felixcheung , @srowen ,

Thanks for suggestions, I've updated the changes and finished the tests.

@SparkQA
Copy link

SparkQA commented Feb 9, 2019

Test build #102112 has finished for PR 23730 at commit b0565e7.

  • This patch fails executing the dev/run-tests script.
  • This patch merges cleanly.
  • This patch adds no public classes.

@cchung100m
update dev/run-tests.py
8fac90d 
Solve ValueError: 'int' is not callable
HyukjinKwon
HyukjinKwon reviewed Feb 9, 2019
View reviewed changes
dev/run-tests.py
if failure_count:
sys.exit(-1)


Copy link
Member

@HyukjinKwon HyukjinKwon Feb 9, 2019

Choose a reason for hiding this comment

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

Let's remove this newline.

Copy link
Author

@cchung100m cchung100m Feb 9, 2019 •
edited
Loading

Choose a reason for hiding this comment

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

Hi @HyukjinKwon

I add the newline to solve PEP8: expected 2 blank lines after class or function definition, found 1, can we keep the newline here?

Copy link
Member

@srowen srowen Feb 9, 2019

Choose a reason for hiding this comment

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

If it's for PEP8, keep it.

Copy link
Member

@HyukjinKwon HyukjinKwon Feb 9, 2019

Choose a reason for hiding this comment

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

Yea that's fine. I wonder why it didn't fail by pep8 check before tho.

@SparkQA
Copy link

SparkQA commented Feb 9, 2019

Test build #102113 has finished for PR 23730 at commit 8fac90d.

  • This patch fails due to an unknown error code, -9.
  • This patch merges cleanly.
  • This patch adds no public classes.

@HyukjinKwon
Copy link
Member

HyukjinKwon commented Feb 9, 2019

retest this please

@SparkQA
Copy link

SparkQA commented Feb 9, 2019

Test build #102115 has finished for PR 23730 at commit 8fac90d.

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

@SparkQA
Copy link

SparkQA commented Feb 9, 2019

Test build #102117 has finished for PR 23730 at commit 47e9163.

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

@srowen
Copy link
Member

srowen commented Feb 10, 2019

Merged to master

@srowen srowen closed this in dc46fb7 Feb 10, 2019
@cchung100m
Copy link
Author

cchung100m commented Feb 10, 2019

@srowen , @felixcheung , @HyukjinKwon

Thank you all for helping me to do the work.

Best regards,

jackylee-ch pushed a commit to jackylee-ch/spark that referenced this pull request Feb 18, 2019
@cchung100m
[SPARK-26822] Upgrade the deprecated module 'optparse'
73be103 
Follow the [official document](https://docs.python.org/2/library/argparse.html#upgrading-optparse-code)  to upgrade the deprecated module 'optparse' to  'argparse'.

## What changes were proposed in this pull request?

This PR proposes to replace 'optparse' module with 'argparse' module.

## How was this patch tested?

Follow the [previous testing](apache@7e3eb3c), manually tested and negative tests were also done. My [test results](https://gist.github.com/cchung100m/1661e7df6e8b66940a6e52a20861f61d)

Closes apache#23730 from cchung100m/solve_deprecated_module_optparse.

Authored-by: cchung100m <cchung100m@cs.ccu.edu.tw>
Signed-off-by: Sean Owen <sean.owen@databricks.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@HyukjinKwon HyukjinKwon HyukjinKwon left review comments

@srowen srowen srowen approved these changes

@felixcheung felixcheung felixcheung approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@cchung100m @HyukjinKwon @SparkQA @felixcheung @srowen