Skip to content

dbm: fix bytes handling#9030

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.

Sign up for GitHub

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

Merged
srittau merged 2 commits into from
Nov 1, 2022
Merged

dbm: fix bytes handling #9030

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
0292961
dbm: fix bytes handling
JelleZijlstra Oct 29, 2022
0da70de
add comment, change type
JelleZijlstra Nov 1, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion stdlib/dbm/__init__.pyi
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -6,7 +6,7 @@ from typing_extensions import Literal, TypeAlias
__all__ = ["open", "whichdb", "error"]

_KeyType: TypeAlias = str | bytes
_ValueType: TypeAlias = str | bytes
_ValueType: TypeAlias = str | bytes | bytearray

@srittausrittauOct 31, 2022

Copy link
Copy Markdown
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.

@JelleZijlstraJelleZijlstraNov 1, 2022

Copy link
Copy Markdown
MemberAuthor

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).

_TFlags: TypeAlias = Literal[
"r",
"w",
Expand Down
3 changes: 3 additions & 0 deletions stdlib/dbm/dumb.pyi
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -10,6 +10,9 @@ _ValueType: TypeAlias = str | bytes

error = OSError

# This class doesn't exist at runtime. open() can return an instance of
# any of the three implementations of dbm (dumb, gnu, ndbm), and this
# class is intended to represent the common interface supported by all three.
class _Database(MutableMapping[_KeyType, bytes]):
def __init__(self, filebasename: str, mode: str, flag: str = ...) -> None: ...
def sync(self) -> None: ...
Expand Down
8 changes: 4 additions & 4 deletions stdlib/dbm/gnu.pyi
View file
Open in desktop
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,13 @@
import sys
from _typeshed import Self
from _typeshed import ReadOnlyBuffer, Self
from types import TracebackType
from typing import TypeVar, overload
from typing_extensions import TypeAlias

if sys.platform != "win32":
_T = TypeVar("_T")
_KeyType: TypeAlias = str | bytes
_ValueType: TypeAlias = str | bytes
_KeyType: TypeAlias = str | ReadOnlyBuffer
_ValueType: TypeAlias = str | ReadOnlyBuffer

open_flags: str

Expand All@@ -31,7 +31,7 @@ if sys.platform != "win32":
@overload
def get(self, k: _KeyType) -> bytes | None: ...
@overload
def get(self, k: _KeyType, default: bytes | _T) -> bytes | _T: ...
def get(self, k: _KeyType, default: _T) -> bytes | _T: ...
def keys(self) -> list[bytes]: ...
def setdefault(self, k: _KeyType, default: _ValueType = ...) -> bytes: ...
# Don't exist at runtime
Expand Down
8 changes: 4 additions & 4 deletions stdlib/dbm/ndbm.pyi
View file
Open in desktop
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,13 @@
import sys
from _typeshed import Self
from _typeshed import ReadOnlyBuffer, Self
from types import TracebackType
from typing import TypeVar, overload
from typing_extensions import TypeAlias

if sys.platform != "win32":
_T = TypeVar("_T")
_KeyType: TypeAlias = str | bytes
_ValueType: TypeAlias = str | bytes
_KeyType: TypeAlias = str | ReadOnlyBuffer
_ValueType: TypeAlias = str | ReadOnlyBuffer

class error(OSError): ...
library: str
Expand All@@ -27,7 +27,7 @@ if sys.platform != "win32":
@overload
def get(self, k: _KeyType) -> bytes | None: ...
@overload
def get(self, k: _KeyType, default: bytes | _T) -> bytes | _T: ...
def get(self, k: _KeyType, default: _T) -> bytes | _T: ...
def keys(self) -> list[bytes]: ...
def setdefault(self, k: _KeyType, default: _ValueType = ...) -> bytes: ...
# Don't exist at runtime
Expand Down