| FazBrowse GitHub Viewer | Trending | | Home |
| Tools: [Download Repo ZIP] [Original HTTPS Page] |
Sorry, something went wrong.
|
|
||
| .. exception:: COMError(hresult, text, details) | ||
|
|
||
| Windows only: This non-public exception is raised when a COM method call |
There was a problem hiding this comment.
You can use .. availability:: Windows (but put the directive after the class doc)
Sorry, something went wrong.
There was a problem hiding this comment.
I followed the pattern in ctypes.rst because it frequently used sentences like Windows only: ....
Since this markup wasn't mentioned in the documentation guide, I overlooked it.
Is this a more modern approach?
Sorry, something went wrong.
There was a problem hiding this comment.
It's widely used in the https://docs.python.org/3/library/socket.html module actually. But if it's not the pattern of ctypes maybe it's better to be consistent. We can make a follow-up PR to transform them into directives.
Sorry, something went wrong.
There was a problem hiding this comment.
I removed the Availability directive and reverted to the original "Windows only: ...".
Once this PR is merged, I would like to work for replacing "Foo only: ..." with the .. availability:: Foo directive.
Sorry, something went wrong.
There was a problem hiding this comment.
Referring to the socket.ioctl documentation, it seems that using :platform: Windows might be more appropriate than .. availability:: Windows. What do you think?
Alternatively, could it be that :platform: is intended to be used exclusively in module sections, as described in the documentation guide's "Module-specific markup"?
Sorry, something went wrong.
There was a problem hiding this comment.
.. availability:: is generally the correct one to use. FWIW, we have some tooling to ensure it's in a correct format.
It's OK to not use it in this PR, to be consistent with the rest of the documentation. But, if you're looking for more to do in the ctypes docs, adding availability would make a great follow-up PR :)
Sorry, something went wrong.
| are specified in the :attr:`!argtypes` tuple. | ||
|
|
||
|
|
||
| .. exception:: COMError(hresult, text, details) |
There was a problem hiding this comment.
Should it be placed here or somewhere else? because it cuts the flow of the reading where paramflags is documented afterwards. Maybe we can put it before the functions?
Sorry, something went wrong.
There was a problem hiding this comment.
I was also unsure whether this was the right place to put it.
On the other hand, placing it above prototype or above FOOFUNCTYPE might seem abrupt.
Might it be appropriate to create the Exceptions section, including ArgumentError?
However, in this scope, I think making such a change that involves the "Foreign functions" section is excessive.
Sorry, something went wrong.
There was a problem hiding this comment.
Well.. honestly I think it makes sense to put it in an exception section. I think it could be fine for this small docs change (I mean, it's for our own convenience). We can ask @hugovk as a docs expert.
Sorry, something went wrong.
There was a problem hiding this comment.
Yes, we often have an exceptions section further down, see for example:
Sorry, something went wrong.
There was a problem hiding this comment.
If the community agrees, creating an "Exceptions" section is no problem at all.
For now, I think I'll try placing it at the end of the document, after the "Arrays and pointers" section.
Sorry, something went wrong.
Co-authored-by: Bénédikt Tran <10796600+picnixz@users.noreply.github.com>
in order to maintain consistency with other sections.
|
AFAICS, COMError is available in _ctypes, but not ctypes. So, documenting it as ctypes.COMError would be misleading. |
Sorry, something went wrong.
|
|
||
| .. attribute:: details | ||
|
|
||
| The 5-tuple representing additional details about the error. |
There was a problem hiding this comment.
That's not very helpful. Could you document what the items are?
Sorry, something went wrong.
There was a problem hiding this comment.
Based on the C implementation source code, these items are retrieved via methods of IErrorInfo1: GetDescription, GetSource, GetHelpFile, GetHelpContext, and the ProgID obtained via using ProgIDFromCLSID and GetGUID.
However, this exception can be raised not only from foreign functions but also intentionally by passing variable-length tuples, as seen in test_win32.py and the comtypes package.
cpython/Lib/test/test_ctypes/test_win32.py
Lines 67 to 76 in f9c5573
I will revise this part, taking these facts into account.
Reference: IErrorInfo implementation in comtypes. ↩
Sorry, something went wrong.
There was a problem hiding this comment.
Since _ctypes is internal API that is subject to change, I think it would be best to change test_win32 and comtypes so that they raise the same 5-tuple as ctypes itself, and document that 5-tuple.
Sorry, something went wrong.
There was a problem hiding this comment.
I wrote details of details.
I plan to improve test_win32 after improving this documentation.
For comtypes, I will investigate how much COMError.details is being used and then discuss the matter with the project's community to decide how to proceed.
As mentioned earlier, I would like to focus on improving the documentation in this PR.
Sorry, something went wrong.
| integer. *name* is name of the COM method. *iid* is an optional pointer to | ||
| the interface identifier which is used in extended error reporting. | ||
|
|
||
| If *iid* is not specified, a :exc:`WindowsError` is raised if the COM method |
There was a problem hiding this comment.
WindowsError is now an alias of OSError.
| If *iid* is not specified, a :exc:`WindowsError` is raised if the COM method | |
| If *iid* is not specified, an :exc:`OSError` is raised if the COM method |
Sorry, something went wrong.
There was a problem hiding this comment.
Good catch.
Thank you.
Sorry, something went wrong.
_CData cannot be imported from either _ctypes or ctypes, but it is documented as "non-public". I consider COMError to be something similar. I DO agree that COMError has value as a public exception. In this PR, I will focus solely on improving the documentation, and I plan to address the new tasks that arise here by opening separate issues. |
Sorry, something went wrong.
|
I see where you're coming from, but I don't think this PR should be merged as-is. It's true that ctypes._CData does not exist as documented. This has been the case since before Python 3. I think that's a bug in the documentation¹⁾; it should not be taken as a precedent. While _CData is private API, but its documentation is useful since it describes public classes that inherit from _CData. But for _ctypes.COMError, I don't see a way to use it without importing it. If you can't use it with except (or isinstance), knowing its attributes is not very helpful. AFAIK, documenting it would only encourage people to use private API. So, I think it needs to become public at the same time as it's documented. ¹⁾ Tangentially: I don't think we have a good way to document private base classes. As with COMType, it would probably be easier to make _CData public than to document it properly as is. |
Sorry, something went wrong.
|
Understood. If the maintainers approve making such a private API public, I have no strong objections. I initially aimed to update the documentation first because I thought proposals to change production code would require extensive discussions from various perspectives and take longer to merge. I’ll add from _ctypes import COMError line and the platform bridge to ctypes/__init__.py. Since this is no longer an internal change, I’ll also add a NEWS entry. Additionally, I’ll include a .. versionchanged:: directive to indicate that while this has existed as a private API in _ctypes, it became public starting with Python 3.14. |
Sorry, something went wrong.
There was a problem hiding this comment.
Thank you!
I do think this is a better way to do it -- if we want to talk about something in the docs, it's better if it has a public name.
Two more nitpicks:
Sorry, something went wrong.
| .. versionchanged:: 3.14 | ||
|
|
||
| This exception is now public. | ||
| Previously, this was private and only available in ``_ctypes``. |
There was a problem hiding this comment.
Let's not expose private API in the docs.
| .. versionchanged:: 3.14 | |
| This exception is now public. | |
| Previously, this was private and only available in ``_ctypes``. | |
| .. versionadded:: next |
Sorry, something went wrong.
There was a problem hiding this comment.
You’re right, this information isn’t particularly useful in the documentation.
I’ll move it to the NEWS file as a supplement.
Sorry, something went wrong.
| the interface identifier which is used in extended error reporting. | ||
|
|
||
| If *iid* is not specified, an :exc:`OSError` is raised if the COM method | ||
| call fails. If *iid* is specified, a :exc:`.COMError` is raised instead. |
There was a problem hiding this comment.
I think (but I'm not 100% sure) that .COMError means Sphinx will search all namespaces, not just ctypes, for a COMError. So it would be better to use ctypes.COMError explicitly:
| call fails. If *iid* is specified, a :exc:`.COMError` is raised instead. | |
| call fails. If *iid* is specified, a :exc:`~ctypes.COMError` is raised instead. |
Sorry, something went wrong.
There was a problem hiding this comment.
Thank you!
Sorry, something went wrong.
Co-authored-by: Bénédikt Tran <10796600+picnixz@users.noreply.github.com>
| Back | FazBrowse Home | New Git URL |
Please refer also to gh-126384 and gh-126610 for the behavior when a foreign function fails to call a COM method.
📚 Documentation preview 📚: https://cpython-previews--126686.org.readthedocs.build/