Skip to content
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.

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

bpo-30421: Added fromfile_parent_relative parameter + docs to argparse.ArgumentParser #1698

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

bob1de
Copy link

@bob1de bob1de commented May 21, 2017

Hi

This is a more enhanced version of http://bugs.python.org/issue30421.

It maintains backwards compatibility and docs have been updated as well.

Just for reference, here is the text of my original issue on BPO.

When one includes an argument file at the command line using argparse and that file contains another include statement, the inner include path is treated as relative to os.getcwd(), what is not the way people expect it to be, I guess.

This patch modifies argparse so that it treats paths of files to include as relative to the location of the file the include was made from.

I also pulled statements that I think should not be enclosed by the try/except out of it and changed the workflow so that the file descriptor of an include file is closed before another one is opened by the recursive call to _read_args_from_files again.. That way, the number of file descriptors open at a time is kept low.

Best regards
Robert

https://bugs.python.org/issue30421

@the-knights-who-say-ni
Copy link

@the-knights-who-say-ni the-knights-who-say-ni commented May 21, 2017

Hello, and thanks for your contribution!

I'm a bot set up to make sure that the project can legally accept your contribution by verifying you have signed the PSF contributor agreement (CLA).

Unfortunately our records indicate you have not signed the CLA. For legal reasons we need you to sign this before we can look at your contribution. Please follow the steps outlined in the CPython devguide to rectify this issue.

Thanks again to your contribution and we look forward to looking at it!

@mention-bot
Copy link

@mention-bot mention-bot commented May 21, 2017

@efficiosoft, thanks for your PR! By analyzing the history of the files in this pull request, we identified @benjaminp, @birkenfeld and @ezio-melotti to be potential reviewers.

@bob1de bob1de changed the title BPO-30421: Added fromfile_parent_relative parameter + docs to argparse.ArgumentParser BPO-30421 Added fromfile_parent_relative parameter + docs to argparse.ArgumentParser May 21, 2017
@bob1de bob1de changed the title BPO-30421 Added fromfile_parent_relative parameter + docs to argparse.ArgumentParser bpo-30421: Added fromfile_parent_relative parameter + docs to argparse.ArgumentParser May 21, 2017
@bob1de bob1de force-pushed the BPO-30421-argparse_fromfile_parent_relative branch from 5cdac77 to 43a13d2 Compare May 24, 2017
@bob1de
Copy link
Author

@bob1de bob1de commented May 24, 2017

Ok, CLA is signed now and I'm ready for feedback.

Best regards
Robert

@bob1de
Copy link
Author

@bob1de bob1de commented Aug 22, 2017

I wanted to ask for some progress on this issue. Is there somebody willing to review it? It still merges without conflicts.

Best regards
Robert

@terryjreedy
Copy link
Member

@terryjreedy terryjreedy commented Aug 28, 2017

Adding a new parameter is an enhancement that has not been mentioned on the issue, let alone approved.

@bob1de
Copy link
Author

@bob1de bob1de commented Aug 28, 2017

Sorry, but what did you mean by "let alone approved"?

Just for reference, here is a quote of the docs for the new parameter of argparse.ArgumentParser() I suggest:

  • fromfile_parent_relative - Whether to treat paths of argument
    files as relative to the location of the file they are specified in
    (True) or to the current working directory (False) (default:
    False)

Best regards
Robert

@terryjreedy
Copy link
Member

@terryjreedy terryjreedy commented Aug 28, 2017

My comment (which constituted my review) was in response to that. You propose to add a new feature with a specific name. AFAIK, neither Bethard nor any other core developer has approved of either the new feature as a concept or the specific name.

To seek such approval, you could post to python-ideas list. (I think most proposals other than for tkinter and IDLE should start there.) Briefly describe the idea and rationale and reference the issue and this PR. If you get a positive response, expect discussion of the option (parameter) name.

@bob1de
Copy link
Author

@bob1de bob1de commented Aug 29, 2017

Thank you for your explanation. This is my first contribution to cpython, hence thinks went a bit wrong somehow.

@bob1de
Copy link
Author

@bob1de bob1de commented Aug 29, 2017

@terryjreedy Ah! I didn't get your response at python-dev. That's why your comment here seemed out-of-context to me. Now I read it and things are clear. Thanks again!

@github-actions
Copy link

@github-actions github-actions bot commented Feb 19, 2022

This PR is stale because it has been open for 30 days with no activity.

@github-actions github-actions bot added the stale label Feb 19, 2022
@MaxwellDupre
Copy link
Contributor

@MaxwellDupre MaxwellDupre commented Mar 14, 2022

Got issue testing so did a full test then got:
422 tests OK.

2 tests failed:
test_embed test_ossaudiodev

9 tests skipped:
test_devpoll test_ioctl test_kqueue test_msilib test_startfile
test_winconsoleio test_winreg test_winsound test_zipfile64

Total duration: 7 min 26 sec
Tests result: FAILURE

cpython on  main [$] via 🐍 v3.11.0a6+ took 7m27s
✦ ❯ gh pr checkout 1698
Switched to branch 'BPO-30421-argparse_fromfile_parent_relative'

cpython on  BPO-30421-argparse_fromfile_parent_relative [$?] via 🐍 v3.11.0a6+ took 2s
✦ ❯ ./python -m test -vvv test_argparse
Error processing line 1 of /home/me/.local/lib/python3.11/site-packages/sphinxcontrib_applehelp-1.0.2-py3.8-nspkg.pth:

Fatal Python error: init_import_site: Failed to import the site module
Python runtime state: initialized
Traceback (most recent call last):
File "", line 186, in addpackage
File "", line 1, in
File "/home/me/Documents/cpython/Lib/importlib/init.py", line 51, in
_w_long = _bootstrap_external._w_long
^^^^^^^^^^^^^^^^^^^^^^^^^^^
AttributeError: module 'importlib._bootstrap_external' has no attribute '_w_long'

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
File "", line 1178, in _find_and_load
File "", line 1149, in _find_and_load_unlocked
File "", line 690, in _load_unlocked
File "", line 982, in exec_module
File "", line 616, in
File "", line 602, in main
File "", line 343, in addusersitepackages
File "", line 226, in addsitedir
File "", line 196, in addpackage
File "/home/me/Documents/cpython/Lib/traceback.py", line 5, in
import linecache
^^^^^^^^^^^^^^^^
File "/home/me/Documents/cpython/Lib/linecache.py", line 11, in
import tokenize
^^^^^^^^^^^^^^^
File "/home/me/Documents/cpython/Lib/tokenize.py", line 33, in
import re
^^^^^^^^^
File "/home/me/Documents/cpython/Lib/re.py", line 123, in
import sre_compile
^^^^^^^^^^^^^^^^^^
File "/home/me/Documents/cpython/Lib/sre_compile.py", line 17, in
assert _sre.MAGIC == MAGIC, "SRE module mismatch"
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
AssertionError: SRE module mismatch

@github-actions github-actions bot removed the stale label Mar 15, 2022
@github-actions
Copy link

@github-actions github-actions bot commented Apr 14, 2022

This PR is stale because it has been open for 30 days with no activity.

@github-actions github-actions bot added the stale label Apr 14, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
awaiting review CLA signed stale
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants