[WIP] gh-90016: Reword sqlite3 adapter/converter docs #93095
Conversation
|
Step one of gh-90016: reword adapter/converter section and add recipes. The latter point is very much a work in progress; I will incorporate some of Serhiy's suggestions here before marking this as ready for review. We will backport this to 3.11 and 3.10, and then deprecate the default adapters/converters (in main only). |
|
I also took the liberty of normalising the argument spec of |
|
For new reviewers: suggestions to how to help reviewing this PR:
|
Modules/_sqlite/module.c
Outdated
| type as tp: unicode | ||
| callable as converter: object |
There was a problem hiding this comment.
Why? The former names looked cleaner to me, and type can be confused with actual type, as in register_adapter().
There was a problem hiding this comment.
What about typename? I'll also revert the second argument back to converter.
There was a problem hiding this comment.
Adressed in 82cf3e2. I changed type to typename. IMO, that's an improvement.
Modules/_sqlite/module.c
Outdated
| @@ -108,16 +108,16 @@ pysqlite_complete_statement_impl(PyObject *module, const char *statement) | |||
| _sqlite3.register_adapter as pysqlite_register_adapter | |||
| type: object(type='PyTypeObject *') | |||
| caster: object | |||
| callable as adapter: object | |||
There was a problem hiding this comment.
Why do you change both the Python and the C names?
The syntax "as" in Argument Clinic was introduced for the case if the Python name and the C name are different and you do not want to rewrite existing C code (or cannot if it is a reserved word). If you change the C name in any case, there is no need in "as".
There was a problem hiding this comment.
The idea was that the Python argument spec used normalised names (type, callable) for both adapters and converters, but the C code used more direct names, as I found it slightly more readable. However, it does add little value, now that I look at it again. (It also helps with new eyes on the code.)
I still want to normalise the Python argument spec in some kind of way, but it is not very important for me, I can leave it out. Currently register_adapter uses (type, caster), and register_converter uses (name, converter).
There was a problem hiding this comment.
Adressed in 82cf3e2.
| *detect_types* defaults to 0 (type detection disabled). | ||
| Set it to any combination of :const:`PARSE_DECLTYPES` and | ||
| :const:`PARSE_COLNAMES` to enable type detection. | ||
| Types cannot be detected for generated fields (for example ``max(data)``), |
There was a problem hiding this comment.
Is it only related to PARSE_DECLTYPES or to PARSE_COLNAMES too? Does SELECT max(data) as "max(data) [integer]" work?
There was a problem hiding this comment.
I would expect it to work, but I haven't tried. EDIT: It works.
There was a problem hiding this comment.
Anyway, I'm not sure that sentence adds real value for the user. IMO, the documentation of the flags says what's needed to know about this.
Doc/library/sqlite3.rst
Outdated
| SQLite's supported types. The callable *callable* accepts as single parameter | ||
| the Python value, and must return a value of the following types: int, | ||
| float, str or bytes. | ||
| Register *callable* to adapt Python *type* into an SQLite type. |
There was a problem hiding this comment.
My English is poor, but the former wording looks more grammatically correct to me. Emphasized *callable* and *type* are names of parameters. Either callable *callable* or a callable.
There was a problem hiding this comment.
I'll come up with something. Thanks!
There was a problem hiding this comment.
Reworded in 82cf3e2
| / | ||
| Registers an adapter with sqlite3's adapter registry. | ||
| Register a function to adapt Python types to SQLite types. |
There was a problem hiding this comment.
Maybe this is better:
| Register a function to adapt Python types to SQLite types. | |
| Register a function to adapt Python objects to SQLite values. |
| / | ||
| Registers a converter with sqlite3. | ||
| Register a function to convert SQLite types to Python types. |
There was a problem hiding this comment.
Maybe this is better:
| Register a function to convert SQLite types to Python types. | |
| Register a function to convert SQLite values to Python objects. |
- revert most argument spec changes - align argument spec across docstrings/docs - improve wording of register_adapter/register_converter - undo note
No description provided.