Skip to content

Conversation

@JelleZijlstra
Copy link
Member

No description provided.

@github-actions

This comment has been minimized.


_KeyType: TypeAlias = str | bytes
_ValueType: TypeAlias = str | bytes
_ValueType: TypeAlias = str | bytes | bytearray
Copy link
Collaborator

Choose a reason for hiding this comment

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

This alias is only used in _Database.__setitem__() as far as I can see (unless the definition is imported elsewhere).

value is parsed using s#, so I think this should be ReadOnlyBuffer:

https://github.com/python/cpython/blob/c1c3be0f9dc414bfae9a5718451ca217751ac687/Modules/_dbmmodule.c#L196

Alternatively, if we want to use the same type for all database types (which is reasonable), I think we should define _ValueType centrally and reuse it.

Copy link
Member Author

Choose a reason for hiding this comment

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

It's a bit more complicated than that. The C code you point to is for dbm.ndbm. The class at issue in this file, dbm._Database, doesn't exist at runtime. We use it as the return type for dbm.open, which can return an instance of any of the database classes in the dbm package. I think the intent is that dbm._Database reflects the common interface of all three. Maybe we should use a Union return type instead?

I think for now I'll switch it to str | bytes, which are the types that all three dbm implementations support. (dbm.dumb is str | bytes | bytearray, the other two are str | ReadOnlyBuffer).

@github-actions
Copy link
Contributor

github-actions bot commented Nov 1, 2022

According to mypy_primer, this change has no effect on the checked open source code. 🤖🎉

@srittau srittau merged commit 3e46f9a into python:master Nov 1, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants