On 1/27/2021 7:01 PM, Guido van Rossum wrote:
It's likely that the additions are going to break someone's code;

Since at least 2007, when Georg moved the 3i reST tree into the 3k branch, the instances have been described as "Class/Function Descriptor Objects", returned by readmodule(_ex), that "provide the following data members". There are no methods (maybe __repr__ should be added for debugging), so these are pure data classes, identical in purpose to pure dataclasses.

The only intended reason to call these would be to create a tree for testing. We do this indirectly with a couple of private test-only pyclbr functions. Being private, *those* functions can have the new parameter added anywhere, regardless of the Class/Function signatures.

test_pyclbr uses a constructed tree as the expected output from readline_ex(test_code). IDLE's test_browser uses a construction as test input to the browser widget. If someone does similar testing, it could break with the proposed changes.

the question seems to be do we care, since the constructors are undocumented?

If any library functions return dataclasses, are the signatures of the (possibly generated) __init__ functions documented?

I care more about breaking tests than possibly non-existent 'off-label' uses, but Python won't know.

But shouldn't we just document the signatures,

The existing or proposed signatures?

so that in the future we're not going to make the same mistake?

I am not sure what you mean by the 'mistake' to not be repeated. I consider it be had been either not declaring the signature private or at least not making optional parameters keyword only.

I also wonder if some judicious use of keyword-only args might help users who currently call the old constructors catch the breakage where it's easily diagnosed (i.e. at the call site) instead of once the object is instantiated (e.g. "why is end_lineno/parent the wrong type")?

With 'end_lineno' added after existing required parameters, just before the existing optional parameter 'parent', an existing Class call ending with '..., posint, parent=SomeClass)' would fail with

TypeError: Class.__init__ missing 1 required positional argument: 'end_lineno'

If parent had previously been made 'keyword only', a call ending instead with '..., num, SomeClass' would be a failure now. But since not,

Perhaps even add some type checks to catch obvious mistakes (could be as simple as `assert isinstance(end_lineno, int)").

Do you consider this enough to make the proposed positioning of 'end_lineno' acceptible? The tradeoff is some immediate annoyance versus forever annoyance of a misplaced required 'optional' parameter.

Since I have in mind a possible IDLE use for end_lineno, quite different from that of the patch author, I consider either way of adding it superior to never adding it.


On Wed, Jan 27, 2021 at 2:36 PM Terry Reedy <tjre...@udel.edu <mailto:tjre...@udel.edu>> wrote:

    pyclbr is the stdlib module browser (once just class browser, hence the
    name).  The doc
    https://docs.python.org/3/library/pyclbr.html#module-pyclbr
    <https://docs.python.org/3/library/pyclbr.html#module-pyclbr>, which I
    revised in 2017, documents readline and readline_ex as the public call
    interface.  The functions return a hierarchical structure that includes
    Function and Class instances.  (Both subclass pyclbr._Object.)  The doc
    I revised already omitted signatures for these classes and just listed
    the attributes of instances.  Just before that listing, I added "Users
    are not expected to create instances of these classes."

    https://bugs.python.org/issue38307
    <https://bugs.python.org/issue38307> and
    https://github.com/python/cpython/pull/24348
    <https://github.com/python/cpython/pull/24348>
    propose to add an end_lineno attribute. Since pyclbr is now, since last
    November, based on the ast tree and since the relevant ast nodes
    have an
    end_line attribute, the proposal amounts to copying that attribute,
    along with others, instead of ignoring it.

    The patch proposes to add 'end_lineno' after existing start 'lineno' in
    the _Object, Function, and Class signatures.  I prefer doing this,
    rather than adding the new parameter at the end of the list, because a)
    being the the logical place would make the code easier to read, and b)
    new names as the end of the signature, follow optional arguments, would
    have to be optional, whereas

    My question is whether the omission of signatures and my added sentence
    sufficiently defines the signatures of these classes (in particular,
    the
    argument order) as private to approve the patch as is?


-- Terry Jan Reedy
    _______________________________________________
    Python-Dev mailing list -- python-dev@python.org
    <mailto:python-dev@python.org>
    To unsubscribe send an email to python-dev-le...@python.org
    <mailto:python-dev-le...@python.org>
    https://mail.python.org/mailman3/lists/python-dev.python.org/
    <https://mail.python.org/mailman3/lists/python-dev.python.org/>
    Message archived at
    
https://mail.python.org/archives/list/python-dev@python.org/message/SQWNNGY7WADHFGAIQZRIMPPLYJGIV4TZ/
    
<https://mail.python.org/archives/list/python-dev@python.org/message/SQWNNGY7WADHFGAIQZRIMPPLYJGIV4TZ/>
    Code of Conduct: http://python.org/psf/codeofconduct/
    <http://python.org/psf/codeofconduct/>



--
--Guido van Rossum (python.org/~guido <http://python.org/~guido>)
/Pronouns: he/him //(why is my pronoun here?)/ <http://feministing.com/2015/02/03/how-using-they-as-a-singular-pronoun-can-change-the-world/>



--
Terry Jan Reedy

_______________________________________________
Python-Dev mailing list -- python-dev@python.org
To unsubscribe send an email to python-dev-le...@python.org
https://mail.python.org/mailman3/lists/python-dev.python.org/
Message archived at 
https://mail.python.org/archives/list/python-dev@python.org/message/Y5RGC4YKFGV7JUY5L4P4PA2VL2376LSU/
Code of Conduct: http://python.org/psf/codeofconduct/

Reply via email to