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

Reformat paragraphs, add backquotes, and add directives to (parts of) ctypes doc. #127210

Merged

Conversation

junkmd
Copy link
Contributor

@junkmd junkmd commented Nov 24, 2024

In gh-127099, I reverted the reformatting paragraphs, leaving the lines in an unorganized state.

I noticed that in the recent documentation, certain parts that should be enclosed in backquotes or use directives were not properly formatted in the ctypes documentation.

I found similar parts in other sections, but for now, I focused on improving only the sections related to gh-127099.


📚 Documentation preview 📚: https://cpython-previews--127210.org.readthedocs.build/

@bedevere-app bedevere-app bot added docs Documentation in the Doc dir skip news awaiting review labels Nov 24, 2024
Copy link
Contributor

@StanFromIreland StanFromIreland left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good consistency changes.

Copy link
Member

@encukou encukou left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks.
For the record: if the lines are shorter than 80 characters, it's not necessary to do any line-wrapping. It's OK to do it as part of a bigger change, though.

Small changes add noise to Git history, and more importantly, any change means the text needs to be re-translated for non-English versions of the docs. (It will appear in English before it's re-translated, see for example Japanese).

So, I'd like to ask: would you be willing to improve this documentation even more? As a maintainer of a major COM library, you are probably the best person to add things like:

  • Stable links to official DllCanUnloadNow & DllGetClassObject documentation
  • Correct documentation for the arguments and return values (including, for example, how to return a S_FALSE from Python -- if we even have public API for that).

error code is specified, the last error code is used by calling the Windows
api function GetLastError.
Returns a textual description of the error code *code*. If no error code is
specified, the last error code is used by calling the Windows api function
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
specified, the last error code is used by calling the Windows api function
specified, the last error code is used by calling the Windows API function

Comment on lines 2145 to 2148
This function is probably the worst-named thing in ``ctypes``. It creates an
instance of :exc:`OSError`. If *code* is not specified, :func:`GetLastError`
is called to determine the error code. If *descr* is not specified,
:func:`FormatError` is called to get a textual description of the error.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since Python 3.3 merged WindowsError into OSError, this is no longer the worst-named thing in ctypes (if it ever was) :)
And the negativity isn't helping anyone.

Suggested change
This function is probably the worst-named thing in ``ctypes``. It creates an
instance of :exc:`OSError`. If *code* is not specified, :func:`GetLastError`
is called to determine the error code. If *descr* is not specified,
:func:`FormatError` is called to get a textual description of the error.
Creates an
instance of :exc:`OSError`. If *code* is not specified, :func:`GetLastError`
is called to determine the error code. If *descr* is not specified,
:func:`FormatError` is called to get a textual description of the error.

@junkmd
Copy link
Contributor Author

junkmd commented Nov 25, 2024

would you be willing to improve this documentation even more?

Sure. This documentation contains dozens of "ctypes" without backquotes. Should those also be included in this PR? <- edited: Considering the impact on documentation in languages other than English, I reconsidered and decided against adding a large number of backquotes.

  • Correct documentation for the arguments and return values (including, for example, how to return a S_FALSE from Python -- if we even have public API for that).

I’m currently looking for appropriate examples. I’ll mention you once the sentence is finalized. Please give me some time.

@junkmd junkmd force-pushed the ctypes_doc_paragraphs_directives_backquotes branch from 4e85cfc to 1f0450b Compare November 25, 2024 14:07
@junkmd junkmd force-pushed the ctypes_doc_paragraphs_directives_backquotes branch from 1f0450b to 0a33f3e Compare November 25, 2024 14:17
@junkmd
Copy link
Contributor Author

junkmd commented Nov 26, 2024

ctypes.DllCanUnloadNow calls the DllCanUnloadNow function from comtypes.

def DllCanUnloadNow():
try:
ccom = __import__("comtypes.server.inprocserver", globals(), locals(), ['*'])
except ImportError:
return 0 # S_OK
return ccom.DllCanUnloadNow()

Similarly, ctypes.DllGetClassObject calls the DllGetClassObject function from comtypes.

def DllGetClassObject(rclsid, riid, ppv):
try:
ccom = __import__("comtypes.server.inprocserver", globals(), locals(), ['*'])
except ImportError:
return -2147221231 # CLASS_E_CLASSNOTAVAILABLE
else:
return ccom.DllGetClassObject(rclsid, riid, ppv)

It seems that for many years, there has been this state where the standard library ctypes depends on the third-party comtypes.

In fact, I have never directly called these functions myself. I am currently investigating how they behave in Python.

Depending on the results of my investigation, I may open an issue regarding these two functions and possibly revert the documentation changes related to them in this PR.

@encukou
Copy link
Member

encukou commented Nov 27, 2024

Oh. I should have checked the implementation!

IMO, we should deprecate those, tell people to import them from comtypes.server.inprocserver instead, and plan to remove them around Python 3.18. Ideally, you'd document them in comtypes instead :)
Would you be OK with that? If so I can write the PR for it.

@junkmd
Copy link
Contributor Author

junkmd commented Nov 27, 2024

IMO, we should deprecate those, tell people to import them from comtypes.server.inprocserver instead, and plan to remove them around Python 3.18. Ideally, you'd document them in comtypes instead :)
Would you be OK with that? If so I can write the PR for it.

I have no objections to deprecating these APIs in ctypes.
Since this is neither an internal nor a small change, I believe it would also be necessary to open an issue.
I would appreciate it if you do that.

On the other hand, I haven’t been able to find any use cases where these functions are called directly.
While I have used the client-side functionality of comtypes, I have rarely utilized the server-side.
As a result, I am unsure how to approach documenting or testing these functions within comtypes.

I think it would be helpful if we could create a flow where developers who see the warning message triggered by calling these functions reach out to the maintainers of ctypes or comtypes and share their use cases.

@junkmd
Copy link
Contributor Author

junkmd commented Nov 27, 2024

For now, I have reverted the documentation for DllCanUnloadNow and DllGetClassObject in this PR.

@junkmd
Copy link
Contributor Author

junkmd commented Nov 27, 2024

It might be worth considering gathering opinions not only through this PR and issue but also on Discourse.

@encukou encukou merged commit 49fee59 into python:main Nov 28, 2024
31 checks passed
@junkmd junkmd deleted the ctypes_doc_paragraphs_directives_backquotes branch November 28, 2024 23:43
@junkmd
Copy link
Contributor Author

junkmd commented Nov 28, 2024

Thank you for merging this.

I am compiling the current state of comtypes and my thoughts in preparation for the discussion in #127369.

Please wait a little.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
docs Documentation in the Doc dir skip issue skip news
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

3 participants