classification
Title: argparse: unused method?
Type: Stage: resolved
Components: Library (Lib) Versions: Python 3.3
process
Status: closed Resolution: wont fix
Dependencies: Superseder:
Assigned To: eric.araujo Nosy List: bethard, eric.araujo, geoffreyspear, madison.may, petri.lehtinen, tshepang
Priority: normal Keywords: patch

Created on 2012-02-21 18:16 by tshepang, last changed 2013-08-31 04:26 by madison.may. This issue is now closed.

Files
File name Uploaded Description Edit
rm-unused-function.patch tshepang, 2012-02-21 18:16 review
Messages (10)
msg153889 - (view) Author: Tshepang Lekhonkhobe (tshepang) * Date: 2012-02-21 18:16
It appears to me that the function I removed (see attached patch) is unused. Since I wasn't sure, I ran the entire test suite, and there wasn't a regression.
msg153897 - (view) Author: Petri Lehtinen (petri.lehtinen) * (Python committer) Date: 2012-02-21 20:24
The _get_args() looks like an obsolete function indeed. However, _AttributeHolder is a base class for other classes, so the function was probably meant to be overridden by a subclass.

Steven probably knows the best whether it should be kept or not.
msg153909 - (view) Author: Steven Bethard (bethard) * (Python committer) Date: 2012-02-21 22:35
Yeah, the intention was that you could just override _get_args in a subclass to get a better __repr__. I think these days all the argparse classes only have keyword arguments, so _get_args probably isn't necessary anymore.
msg154183 - (view) Author: Éric Araujo (eric.araujo) * (Python committer) Date: 2012-02-25 05:36
Following Steven’s message, I will remove the unused function.
msg154269 - (view) Author: Éric Araujo (eric.araujo) * (Python committer) Date: 2012-02-25 16:29
Hm, I was about to commit this and did a double-take.  Are we 100% sure that nobody in the whole wide world of Python does not override this function?  Shouldn’t we first send a DeprecationWarning in 3.3 and remove later?
msg154291 - (view) Author: Steven Bethard (bethard) * (Python committer) Date: 2012-02-26 00:27
It's undocumented, and it begins with an underscore, but it would certainly be safer to deprecate it first.
msg154292 - (view) Author: Geoffrey Spear (geoffreyspear) * Date: 2012-02-26 00:49
If people do override this provate method, would they really bother calling super() to get the essentially no-op functionality of the superclass method? 

FWIW, Google Code and github both seem to be free of any such code.
msg154302 - (view) Author: Éric Araujo (eric.araujo) * (Python committer) Date: 2012-02-26 03:06
> It's undocumented, and it begins with an underscore, but it would certainly be safer
> to deprecate it first.

Ah, somehow I misunderstood your earlier comment and thought _get_args had been publicized.  Removing is safe then.

Geoffrey:
> If people do override this provate method, would they really bother calling super()
> to get the essentially no-op functionality of the superclass method? 
No, but they would expect __repr__ to use their _get_args, which it won’t after the removal (see the patch).

> FWIW, Google Code and github both seem to be free of any such code.
Thanks for checking.  It’s only a fraction of all code out there, but it’s an indicator.
msg161369 - (view) Author: Petri Lehtinen (petri.lehtinen) * (Python committer) Date: 2012-05-22 18:47
Nobody's really sure whether it's safe to remove, so let it be there. Leaving it can be justified by symmetry with _get_kwargs. Closing.
msg196616 - (view) Author: Madison May (madison.may) * Date: 2013-08-31 04:26
Any chance this issue could be reopened?  I ran across this bit of code today when working on coverage for argparse.  I'd like to again propose the removal or modification of _get_args.  I understand that it's there primarily to be overridden, but even that's not useful.  Because _get_kwargs uses obj.__dict__.items(), overriding _get_args to return an iterable of positional args results in those arguments being output twice when repr() is called (once formatted as a positional arg and once formatted as a keyword arg).  You end up with strings like the below.  I just can't think of any situation where this behavior would be desirable.

Action(['--foo', '-a', '-b'], 'b', option_strings=['--foo', '-a', '-b'], dest='b', nargs='+', const=None, default=42, type='int', choices=[1, 2, 3], help='HELP', metavar='METAVAR')
History
Date User Action Args
2013-08-31 04:26:20madison.maysetnosy: + madison.may
messages: + msg196616
2012-05-22 18:47:01petri.lehtinensetstatus: open -> closed
resolution: wont fix
messages: + msg161369

stage: resolved
2012-02-26 03:06:37eric.araujosetmessages: + msg154302
2012-02-26 00:49:29geoffreyspearsetmessages: + msg154292
2012-02-26 00:27:20bethardsetmessages: + msg154291
2012-02-25 16:29:23eric.araujosetmessages: + msg154269
2012-02-25 05:36:13eric.araujosetassignee: eric.araujo

messages: + msg154183
nosy: + eric.araujo
2012-02-21 22:35:24bethardsetmessages: + msg153909
2012-02-21 20:55:21geoffreyspearsetnosy: + geoffreyspear
2012-02-21 20:24:58petri.lehtinensetnosy: + petri.lehtinen
messages: + msg153897
2012-02-21 19:27:03nadeem.vawdasetnosy: + bethard
2012-02-21 18:16:39tshepangcreate