SilentGhost ghost@gmail.com added the comment:
Steven, I'm wondering why do you raise ArgumentTypeError there? From reading
doc strings of the relevant errors, it seems obvious to me that it should be
ArgumentError. Argument is being used there, there's no conversion occurring
anywhere.
Steven Bethard steven.beth...@gmail.com added the comment:
It's an ArgumentTypeError because that's what you're supposed to raise inside
type functions:
http://docs.python.org/dev/library/argparse.html#type
(Note that argparse.FileType.__call__ is what will be called when we pass
Georg Brandl ge...@python.org added the comment:
No, it sounds fine then.
--
___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue9509
___
___
Steven Bethard steven.beth...@gmail.com added the comment:
Fixed in r88169 and r88171. Thanks everyone for your help! I'll be keeping my
eye on the buildbots for a bit to make sure everything stays green.
--
assignee: - bethard
resolution: - fixed
stage: patch review -
Éric Araujo mer...@netwok.org added the comment:
Oops, I missed that while reading the patch: It introduces a translatable
string which may cause i18n issues (discussed in #10528). I can propose a
patch this week if no-one beats me to it.
--
___
Steven Bethard steven.beth...@gmail.com added the comment:
Georg, is this something we can patch for rc2? It's a bug - errors encountered
by argparse-internal code should be translated into command line errors, and
they currently aren't for read-only files.
For what it's worth, the tests fail
Georg Brandl ge...@python.org added the comment:
This would be acceptable to patch; I wonder whether to swallow the exception
text of IOError though.
--
___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue9509
Steven Bethard steven.beth...@gmail.com added the comment:
Good point. Here's the updated patch that reports the IOError as well. All
tests pass. I'll apply in a bit if I don't hear otherwise.
--
Added file: http://bugs.python.org/file20491/argparse.diff
Georg Brandl ge...@python.org added the comment:
Library patch looks good.
Tests: Does the create_readonly_file helper work on all platforms, esp.
Windows? Maybe it's better to create a different error situation?
--
___
Python tracker
SilentGhost ghost@gmail.com added the comment:
I've tested this on windows. It passed all test.
--
___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue9509
___
Steven Bethard steven.beth...@gmail.com added the comment:
The docs for os.chmod claim:
Availability: Unix, Windows.
Although Windows supports chmod(), you can only set the file's read-only flag
with it (via the stat.S_IWRITE and stat.S_IREAD constants or a corresponding
integer value). All
Changes by SilentGhost ghost@gmail.com:
--
status: open - languishing
___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue9509
___
___
Éric Araujo mer...@netwok.org added the comment:
SilentGhost, can you remove old files from the report (or tell which ones I
should remove if you can’t do it) and make one patch with code and test changes
that apply cleanly to current py3k? That would make a final review by Steven
easier.
SilentGhost ghost@gmail.com added the comment:
Here is the single patch. All tests pass.
--
Added file: http://bugs.python.org/file20456/argparse.diff
___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue9509
Changes by SilentGhost ghost@gmail.com:
Removed file: http://bugs.python.org/file19827/test_argparse.py.diff
___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue9509
___
Changes by SilentGhost ghost@gmail.com:
Removed file: http://bugs.python.org/file19805/argparse.py.diff
___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue9509
___
akira 4kir4...@gmail.com added the comment:
no such file or directory '%s' is misleading if you are trying to open a
readonly file for writing.
The message might be replace by: can't open '%s'
--
___
Python tracker rep...@bugs.python.org
Changes by SilentGhost ghost@gmail.com:
Added file: http://bugs.python.org/file20458/argparse.diff
___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue9509
___
Changes by SilentGhost ghost@gmail.com:
Removed file: http://bugs.python.org/file20456/argparse.diff
___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue9509
___
Steven Bethard steven.beth...@gmail.com added the comment:
Tried to comment in Rietveld but it didn't work for some reason. Anyway, I
think the argparse.py patch isn't good - changing the type error message to
'invalid %s value: %r details: %s' will change the behavior of a bunch of
programs,
SilentGhost michael.mischurow+...@gmail.com added the comment:
Steven, I'm not sure why you're insisting on ArgumentTypeError, when it should
be ArgumentError. The file name is not coerced into a different file type, but
rather the error occurs when trying to use parameter passed.
In any way,
Steven Bethard steven.beth...@gmail.com added the comment:
Sorry, I was looking at the akira patch with the same date, where I was mainly
worried about the modification of the except (TypeError, ValueError): block.
Your patch doesn't do that, and looks fine.
--
Changes by SilentGhost michael.mischurow+...@gmail.com:
Removed file: http://bugs.python.org/file19815/test_argparse.py.diff
___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue9509
___
SilentGhost michael.mischurow+...@gmail.com added the comment:
On windows proposed changes to Lib/test/test_argparse.py cause it to enter an
infinite loop in TempDirMixin.tearDown method.
As it seemed exclusively Windows issue, this new patch replaces while loop with
the ignore_errors
SilentGhost michael.mischurow+...@gmail.com added the comment:
Ammended akira's patch for Lib/test/test_argparse.py to include suggested in
review changes: with statement, import statement
--
Added file: http://bugs.python.org/file19815/test_argparse.py.diff
Steven Bethard steven.beth...@gmail.com added the comment:
I'm not sure about the patch - this will convert *all* IOErrors into command
line error messages, while we should really only be converting the ones raised
by FileType. Instead, the try/except should be in FileType.__call__, and you
SilentGhost michael.mischurow+...@gmail.com added the comment:
Attached patch with the try-except clause as suggested by Steven, Doug's
example now produces the following output:
$ ./python argparse_filetype_error.py
usage: argparse_filetype_error.py [-h] [-i I]
argparse_filetype_error.py:
akira 4kir4...@gmail.com added the comment:
I've added tests for readonly files. SilentGhost's patch doesn't handle this
case.
--
Added file: http://bugs.python.org/file19806/test_argparse.diff
___
Python tracker rep...@bugs.python.org
akira 4kir4...@gmail.com added the comment:
Attached patch for argparse.py (argparse.diff -- without tests, see tests in
test_argparse.diff) where ValueError is raises in FileType.__call__ and
additional details printed on ArgumentError
--
Added file:
akira 4kir4...@gmail.com added the comment:
updated patch on http://codereview.appspot.com/3251041/
--
___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue9509
___
Tarsis Azevedo tarsis.azev...@gmail.com added the comment:
Hi Doug,
I catched the IOError exception and used the error function of ArgumentParser
class to return a beautful error message.
--
keywords: +patch
nosy: +Tarsis.Azevedo
Added file:
Doug Hellmann doug.hellm...@gmail.com added the comment:
Hi, Tarsis,
That patch looks good to me.
Thanks!
Doug
--
___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue9509
___
Éric Araujo mer...@netwok.org added the comment:
Looks good to me too. Steven, does this require a test?
--
nosy: +eric.araujo
stage: - patch review
___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue9509
Doug Hellmann doug.hellm...@gmail.com added the comment:
Oh, yeah, a test is a good idea.
--
___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue9509
___
akira 4kir4...@gmail.com added the comment:
Simplified the patch and added test for a non-existent file in issue9509.patch
The test `py3k/python -m test.regrtest test_argparse` fails without the patch
and succeeds with it.
The docs in Doc/library/argparse.rst and Lib/argparse.py don't need
Éric Araujo mer...@netwok.org added the comment:
I don’t know enough of the test_argparse magic to assess the new test, but
Stephan, the author and maintainer of argparse, will certainly comment when he
gets some time. Thanks for the patch!
--
___
Changes by R. David Murray rdmur...@bitdance.com:
--
nosy: +bethard
___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue9509
___
___
Python-bugs-list
Changes by Doug Hellmann doug.hellm...@gmail.com:
--
keywords: +easy
___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue9509
___
___
Python-bugs-list
New submission from Doug Hellmann doug.hellm...@gmail.com:
Most of the argparse type converters handle exceptions with a single line error
message explaining the problem. For example, if an argument -i is declared as
an int, but the value given ('a') cannot be converted to an int, the message
Changes by Doug Hellmann doug.hellm...@gmail.com:
Added file: http://bugs.python.org/file18381/argparse_int_error.py
___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue9509
___
40 matches
Mail list logo
