python / cpython

The Python programming language
https://www.python.org
Other
63.49k stars 30.41k forks source link

Some problems in documentation extending/newtypes.html #56881

Closed 786d3f11-b763-4414-a03f-abc264e0b72d closed 13 years ago

786d3f11-b763-4414-a03f-abc264e0b72d commented 13 years ago
BPO 12672
Nosy @terryjreedy, @merwok, @ericsnowcurrently

Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.

Show more details

GitHub fields: ```python assignee = None closed_at = created_at = labels = ['docs'] title = 'Some problems in documentation extending/newtypes.html' updated_at = user = 'https://bugs.python.org/elibendersky' ``` bugs.python.org fields: ```python activity = actor = 'python-dev' assignee = 'docs@python' closed = True closed_date = closer = 'eli.bendersky' components = ['Documentation'] creation = creator = 'eli.bendersky' dependencies = [] files = [] hgrepos = [] issue_num = 12672 keywords = [] message_count = 18.0 messages = ['141487', '141788', '141807', '141910', '141943', '141944', '141945', '141961', '141999', '142000', '142088', '142158', '142161', '142162', '142212', '142412', '142413', '143481'] nosy_count = 6.0 nosy_names = ['terry.reedy', 'eric.araujo', 'eli.bendersky', 'docs@python', 'python-dev', 'eric.snow'] pr_nums = [] priority = 'low' resolution = 'fixed' stage = 'patch review' status = 'closed' superseder = None type = None url = 'https://bugs.python.org/issue12672' versions = ['Python 3.3'] ```

786d3f11-b763-4414-a03f-abc264e0b72d commented 13 years ago

Reading the C API documentation: extending/newtypes.html

Some problems:

  1. In the first paragraph of 2.1 - "to distinguish them from things like [].append", it's unclear what [].append is. Maybe "[] and append"?
  2. The C coding convention is unusual and non PEP-7 in 2.1.1 (after "if (self->first == NULL)" in Noddy_new)
  3. The previous problem repeats in other instances of Noddy_new in the document

I found these while reading the 2.7 docs, but they could also exist in 3.x (didn't check).

merwok commented 13 years ago

it's unclear what [].append is. It’s crystal clear to me: [].append is a method of a list object, just created here by a literal. (Maybe you’re not aware that [].append is valid Python.)

The C coding convention is unusual and non PEP-7 in 2.1.1 Running Tools/scripts/untabify.py on the C codebase reveals problems in Doc/include/*.c, Objects, Python, etc.

786d3f11-b763-4414-a03f-abc264e0b72d commented 13 years ago

Éric, I know perfectly well that [].append is valid Python, but I don't think this is the clearest way to give an example of an object method. I think spelling [].append's meaning more explicitly would be better.

I'm also aware that there are tab problems all over the code base. I'm not suggesting a large cleanup. But I do think that in *example code* in the documentation, it wouldn't hurt to make the code idiomatically styled. After all, this is what people will copy-paste when writing new code.

merwok commented 13 years ago

I know perfectly well that [].append is valid Python, but I don't think this is the clearest way to give an example of an object method. I think spelling [].append's meaning more explicitly would be better. Would it be clearer if we replaced the literal with a name?

  These C functions are called “type methods” to distinguish them from
- things like [].append (which we call “object methods”).
+ methods bound to specific instances (things like sys.path.append),
+ which we call “object methods”.

I'm also aware that there are tab problems all over the code base. I'm not suggesting a large cleanup. *I* was suggesting a large cleanup :), but we can do that in another commit. If you want to clean the example code in Doc/extending or even just in newtypes.rst, I think you can just go ahead.

1762cc99-3127-4a62-9baf-30c3d0f51ef7 commented 13 years ago

New changeset 683202530137 by Eli Bendersky in branch 'default': Issue bpo-12672: fix code samples in extending/newtypes.html for PEP-7 compliance http://hg.python.org/cpython/rev/683202530137

786d3f11-b763-4414-a03f-abc264e0b72d commented 13 years ago

Would it be clearer if we replaced the literal with a name?

These C functions are called “type methods” to distinguish them from

  • things like [].append (which we call “object methods”).
  • methods bound to specific instances (things like sys.path.append),
  • which we call “object methods”.

No, I don't think this is the intention (bound vs. unbound). I think the distinction is between special methods recognized by Python, and "plain object methods" defined by the user. Not sure how to express this clearly in the docs though.

Re PEP-7 cleanup: done some for extending/newtypes.html - not sure everything is fixed but it's a bit better now.

786d3f11-b763-4414-a03f-abc264e0b72d commented 13 years ago

Maybe it should say:

"... to distinguish them from custom class methods such as list's append"

I think this is more correct, because it clearly refers to the methods placed in the 'tp_methods' field of a type.

... and also drop the (which we call "object methods") since this terminology isn't actually being used in the rest of the article.

merwok commented 13 years ago

I think the distinction is between special methods recognized by Python, and "plain object methods" defined by the user.

Do you mean __special__ methods? Re-reading the whole paragraph, I can’t tell :(

ericsnowcurrently commented 13 years ago

Eli, I interpreted it the same way you did. In the doc, "type methods" are those that map directly to PyTypeObject. Any custom type methods go in tpmethods. You could almost call the former "PyTypeObject methods" rather than "type methods". And both are distinct from functions/methods in a type's \_dict__...

Also I agree that the "object methods" statement is unnecessary.

ericsnowcurrently commented 13 years ago

http://docs.python.org/dev/extending/newtypes.html

terryjreedy commented 13 years ago

I agree that the sentence is a bit confusing and the 'object method' ambiguous. I suspect that the sentence was written years ago. In current Python, [].append is a bound method of class 'builtin_function_or_method'. I *suspect* that the intended contrast, and certainly the important one, is that between C functions, which get added to PyTypeObject structures, and their Python object wrappers that are visible from Python, but which must not be put in the type structure. The varieties of wrappers are irrelevant in this context and for the purpose of avoiding that mistake. So I would rewrite the sentence as:

These C functions are called “type methods” to distinguish them from Python wrapper objects, such as list.append or [].append, visible in Python code.

Looking further down, "Now if you go and look up the definition of PyTypeObject in object.h you’ll see that it has many more fields that the definition above.", needs 'that' changed to 'than' and I would insert " following tp_doc" after 'fields'.

786d3f11-b763-4414-a03f-abc264e0b72d commented 13 years ago

Terry, I'm not 100% sure about what you mean by "Python wrapper objects ... visible from Python", but I think I'll disagree.

There's a big difference between "C functions" in general and "type methods" this document speaks of. Let's leave list aside for a moment, since its being built-in complicates matters a bit, and let's talk about the "Noddy" type this documentation page plays with.

You may implement "normal" methods for Noddy, such as the "name" method added in the first example, by defining an array of PyMethodDef structures and assigning it to tp_methods.

On the other hand, the other tp fields imlpement special type methods (used by \_new, __str, getattr/setattr, and so on). This is the major difference. Both are C functions, but some implement special type methods and some implement "normal" object methods.

If this is also what you meant, I apologize for disagreeing :-)

I believe my latest rephrasing proposal is reflecting the above understanding.

P.S. as for s/that/than/ further down - good catch, will add it to the patch when we decide about the first issue

terryjreedy commented 13 years ago

"the type object determines which (C) functions get called when, for instance, an attribute gets looked up on an object or it is multiplied by another object. These C functions are called “type methods”

"These C functions" are any of the C functions that are members of the type object. But they are C-level methods.

"to distinguish them from things like [].append (which we call “object methods”)."

[].append is a Python-level method object that wraps a C function.

My revised suggestion is "... in contrast to PyObject that contain C functions, such as list.append or [].append."

The only contrast that makes sense to me in this context is between directly callable C functions and Py_Objects (which have just been described) that contain a C function. I believe that author is addressing Python programmers who are used to 'method' referring to Python objects whereas the author wants to use 'method' to refer to C functions, which are not Python objects.

Or the sentence could be deleted.

786d3f11-b763-4414-a03f-abc264e0b72d commented 13 years ago

"[].append is a Python-level method object that wraps a C function."

What makes you think that? There's no Python implementation of .append that I know of. Neither is there a Python implementation of the Noddy.name method that is discussed in the page. Both are implemented solely in C and exposed as methods for their respective classes via the tp_methods array.

"Or the sentence could be deleted."

This could be problematic, because the document does refer to "type methods" on several occasions, and it makes sense to define what it means.

terryjreedy commented 13 years ago

You are right, I suggested deleting too much. The first half of the sentence is needed to define 'type methods', which is used several more times and is the title of the next section. We need to keep "These C functions are called “type methods”." In the context of the preceding sentence and later usages, I think this is enough.

The second half of the sentence is intended to refine the definition by contrast, but it fails to do so since we cannot agree on what the contrast is. Since none of the interpretations make complete sense and since 'object methods' is not used again, making its definition irrelevant, I suggest deleting this part: "to distinguish them from things like [].append (which we call “object methods”).".

1762cc99-3127-4a62-9baf-30c3d0f51ef7 commented 13 years ago

New changeset d062d482642c by Eli Bendersky in branch '3.2': Issue bpo-12672: remove confusing part of sentence in documentation http://hg.python.org/cpython/rev/d062d482642c

New changeset 558f2270cba8 by Eli Bendersky in branch 'default': Merge from 3.2 http://hg.python.org/cpython/rev/558f2270cba8

1762cc99-3127-4a62-9baf-30c3d0f51ef7 commented 13 years ago

New changeset 893f858c600e by Eli Bendersky in branch '2.7': Issue bpo-12672: remove confusing part of sentence in documentation http://hg.python.org/cpython/rev/893f858c600e

1762cc99-3127-4a62-9baf-30c3d0f51ef7 commented 13 years ago

New changeset 8083e51522ee by Eli Bendersky in branch '3.2': Issue bpo-12672: remove confusing part of sentence in documentation http://hg.python.org/cpython/rev/8083e51522ee