Skip to content

PYTHON-5115 Type validation errors should include the invalid type name #2085

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

Merged
merged 8 commits into from
Jan 31, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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 bson/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -1386,7 +1386,7 @@ def is_valid(bson: bytes) -> bool:
:param bson: the data to be validated
"""
if not isinstance(bson, bytes):
raise TypeError("BSON data must be an instance of a subclass of bytes")
raise TypeError(f"BSON data must be an instance of a subclass of bytes, not {type(bson)}")

try:
_bson_to_dict(bson, DEFAULT_CODEC_OPTIONS)
Expand Down
6 changes: 3 additions & 3 deletions bson/binary.py
Original file line number Diff line number Diff line change
Expand Up @@ -290,7 +290,7 @@ def __new__(
subtype: int = BINARY_SUBTYPE,
) -> Binary:
if not isinstance(subtype, int):
raise TypeError("subtype must be an instance of int")
raise TypeError(f"subtype must be an instance of int, not {type(subtype)}")
if subtype >= 256 or subtype < 0:
raise ValueError("subtype must be contained in [0, 256)")
# Support any type that implements the buffer protocol.
Expand Down Expand Up @@ -321,7 +321,7 @@ def from_uuid(
.. versionadded:: 3.11
"""
if not isinstance(uuid, UUID):
raise TypeError("uuid must be an instance of uuid.UUID")
raise TypeError(f"uuid must be an instance of uuid.UUID, not {type(uuid)}")

if uuid_representation not in ALL_UUID_REPRESENTATIONS:
raise ValueError(
Expand Down Expand Up @@ -470,7 +470,7 @@ def as_vector(self) -> BinaryVector:
"""

if self.subtype != VECTOR_SUBTYPE:
raise ValueError(f"Cannot decode subtype {self.subtype} as a vector.")
raise ValueError(f"Cannot decode subtype {self.subtype} as a vector")

position = 0
dtype, padding = struct.unpack_from("<sB", self, position)
Expand Down
4 changes: 2 additions & 2 deletions bson/code.py
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ def __new__(
**kwargs: Any,
) -> Code:
if not isinstance(code, str):
raise TypeError("code must be an instance of str")
raise TypeError(f"code must be an instance of str, not {type(code)}")

self = str.__new__(cls, code)

Expand All @@ -67,7 +67,7 @@ def __new__(

if scope is not None:
if not isinstance(scope, _Mapping):
raise TypeError("scope must be an instance of dict")
raise TypeError(f"scope must be an instance of dict, not {type(scope)}")
if self.__scope is not None:
self.__scope.update(scope) # type: ignore
else:
Expand Down
12 changes: 9 additions & 3 deletions bson/codec_options.py
Original file line number Diff line number Diff line change
Expand Up @@ -401,17 +401,23 @@ def __new__(
"uuid_representation must be a value from bson.binary.UuidRepresentation"
)
if not isinstance(unicode_decode_error_handler, str):
raise ValueError("unicode_decode_error_handler must be a string")
raise ValueError(
f"unicode_decode_error_handler must be a string, not {type(unicode_decode_error_handler)}"
)
if tzinfo is not None:
if not isinstance(tzinfo, datetime.tzinfo):
raise TypeError("tzinfo must be an instance of datetime.tzinfo")
raise TypeError(
f"tzinfo must be an instance of datetime.tzinfo, not {type(tzinfo)}"
)
if not tz_aware:
raise ValueError("cannot specify tzinfo without also setting tz_aware=True")

type_registry = type_registry or TypeRegistry()

if not isinstance(type_registry, TypeRegistry):
raise TypeError("type_registry must be an instance of TypeRegistry")
raise TypeError(
f"type_registry must be an instance of TypeRegistry, not {type(type_registry)}"
)

return tuple.__new__(
cls,
Expand Down
4 changes: 2 additions & 2 deletions bson/dbref.py
Original file line number Diff line number Diff line change
Expand Up @@ -56,9 +56,9 @@ def __init__(
.. seealso:: The MongoDB documentation on `dbrefs <https://dochub.mongodb.org/core/dbrefs>`_.
"""
if not isinstance(collection, str):
raise TypeError("collection must be an instance of str")
raise TypeError(f"collection must be an instance of str, not {type(collection)}")
if database is not None and not isinstance(database, str):
raise TypeError("database must be an instance of str")
raise TypeError(f"database must be an instance of str, not {type(database)}")

self.__collection = collection
self.__id = id
Expand Down
2 changes: 1 addition & 1 deletion bson/decimal128.py
Original file line number Diff line number Diff line change
Expand Up @@ -277,7 +277,7 @@ def from_bid(cls: Type[Decimal128], value: bytes) -> Decimal128:
point in Binary Integer Decimal (BID) format).
"""
if not isinstance(value, bytes):
raise TypeError("value must be an instance of bytes")
raise TypeError(f"value must be an instance of bytes, not {type(value)}")
if len(value) != 16:
raise ValueError("value must be exactly 16 bytes")
return cls((_UNPACK_64(value[8:])[0], _UNPACK_64(value[:8])[0])) # type: ignore
Expand Down
4 changes: 2 additions & 2 deletions bson/timestamp.py
Original file line number Diff line number Diff line change
Expand Up @@ -58,9 +58,9 @@ def __init__(self, time: Union[datetime.datetime, int], inc: int) -> None:
time = time - offset
time = int(calendar.timegm(time.timetuple()))
if not isinstance(time, int):
raise TypeError("time must be an instance of int")
raise TypeError(f"time must be an instance of int, not {type(time)}")
if not isinstance(inc, int):
raise TypeError("inc must be an instance of int")
raise TypeError(f"inc must be an instance of int, not {type(inc)}")
if not 0 <= time < UPPERBOUND:
raise ValueError("time must be contained in [0, 2**32)")
if not 0 <= inc < UPPERBOUND:
Expand Down
12 changes: 8 additions & 4 deletions gridfs/asynchronous/grid_file.py
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,7 @@ def __init__(self, database: AsyncDatabase, collection: str = "fs"):
.. seealso:: The MongoDB documentation on `gridfs <https://dochub.mongodb.org/core/gridfs>`_.
"""
if not isinstance(database, AsyncDatabase):
raise TypeError("database must be an instance of Database")
raise TypeError(f"database must be an instance of Database, not {type(database)}")

database = _clear_entity_type_registry(database)

Expand Down Expand Up @@ -503,7 +503,7 @@ def __init__(
.. seealso:: The MongoDB documentation on `gridfs <https://dochub.mongodb.org/core/gridfs>`_.
"""
if not isinstance(db, AsyncDatabase):
raise TypeError("database must be an instance of AsyncDatabase")
raise TypeError(f"database must be an instance of AsyncDatabase, not {type(db)}")

db = _clear_entity_type_registry(db)

Expand Down Expand Up @@ -1082,7 +1082,9 @@ def __init__(
:attr:`~pymongo.collection.AsyncCollection.write_concern`
"""
if not isinstance(root_collection, AsyncCollection):
raise TypeError("root_collection must be an instance of AsyncCollection")
raise TypeError(
f"root_collection must be an instance of AsyncCollection, not {type(root_collection)}"
)

if not root_collection.write_concern.acknowledged:
raise ConfigurationError("root_collection must use acknowledged write_concern")
Expand Down Expand Up @@ -1436,7 +1438,9 @@ def __init__(
from the server. Metadata is fetched when first needed.
"""
if not isinstance(root_collection, AsyncCollection):
raise TypeError("root_collection must be an instance of AsyncCollection")
raise TypeError(
f"root_collection must be an instance of AsyncCollection, not {type(root_collection)}"
)
_disallow_transactions(session)

root_collection = _clear_entity_type_registry(root_collection)
Expand Down
12 changes: 8 additions & 4 deletions gridfs/synchronous/grid_file.py
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,7 @@ def __init__(self, database: Database, collection: str = "fs"):
.. seealso:: The MongoDB documentation on `gridfs <https://dochub.mongodb.org/core/gridfs>`_.
"""
if not isinstance(database, Database):
raise TypeError("database must be an instance of Database")
raise TypeError(f"database must be an instance of Database, not {type(database)}")

database = _clear_entity_type_registry(database)

Expand Down Expand Up @@ -501,7 +501,7 @@ def __init__(
.. seealso:: The MongoDB documentation on `gridfs <https://dochub.mongodb.org/core/gridfs>`_.
"""
if not isinstance(db, Database):
raise TypeError("database must be an instance of Database")
raise TypeError(f"database must be an instance of Database, not {type(db)}")

db = _clear_entity_type_registry(db)

Expand Down Expand Up @@ -1076,7 +1076,9 @@ def __init__(
:attr:`~pymongo.collection.Collection.write_concern`
"""
if not isinstance(root_collection, Collection):
raise TypeError("root_collection must be an instance of Collection")
raise TypeError(
f"root_collection must be an instance of Collection, not {type(root_collection)}"
)

if not root_collection.write_concern.acknowledged:
raise ConfigurationError("root_collection must use acknowledged write_concern")
Expand Down Expand Up @@ -1426,7 +1428,9 @@ def __init__(
from the server. Metadata is fetched when first needed.
"""
if not isinstance(root_collection, Collection):
raise TypeError("root_collection must be an instance of Collection")
raise TypeError(
f"root_collection must be an instance of Collection, not {type(root_collection)}"
)
_disallow_transactions(session)

root_collection = _clear_entity_type_registry(root_collection)
Expand Down
2 changes: 1 addition & 1 deletion pymongo/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -160,7 +160,7 @@ def timeout(seconds: Optional[float]) -> ContextManager[None]:
.. versionadded:: 4.2
"""
if not isinstance(seconds, (int, float, type(None))):
raise TypeError("timeout must be None, an int, or a float")
raise TypeError(f"timeout must be None, an int, or a float, not {type(seconds)}")
if seconds and seconds < 0:
raise ValueError("timeout cannot be negative")
if seconds is not None:
Expand Down
2 changes: 1 addition & 1 deletion pymongo/_asyncio_lock.py
Original file line number Diff line number Diff line change
Expand Up @@ -160,7 +160,7 @@ def release(self) -> None:
self._locked = False
self._wake_up_first()
else:
raise RuntimeError("Lock is not acquired.")
raise RuntimeError("Lock is not acquired")

def _wake_up_first(self) -> None:
"""Ensure that the first waiter will wake up."""
Expand Down
2 changes: 1 addition & 1 deletion pymongo/_azure_helpers.py
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ def _get_azure_response(
try:
data = json.loads(body)
except Exception:
raise ValueError("Azure IMDS response must be in JSON format.") from None
raise ValueError("Azure IMDS response must be in JSON format") from None

for key in ["access_token", "expires_in"]:
if not data.get(key):
Expand Down
2 changes: 1 addition & 1 deletion pymongo/asynchronous/auth.py
Copy link
Contributor

Choose a reason for hiding this comment

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

Looks like the changes in this file (and some other asynchronous files) aren't reflected in sync, did you run prepre-commit run --all-files before committing?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yes, I did but not getting any changes in sync files.

$ pre-commit run --all-files
check for added large files..............................................Passed
check for case conflicts.................................................Passed
check toml...............................................................Passed
check yaml...............................................................Passed
debug statements (python)................................................Passed
fix end of files.........................................................Passed
forbid new submodules................................(no files to check)Skipped
trim trailing whitespace.................................................Passed
synchro..................................................................Passed
ruff.....................................................................Passed
ruff-format..............................................................Passed
blacken-docs.............................................................Passed
rst code is two backticks............................................Passed
rst directives end with two colons.......................................Passed
rst inline code next to normal text..................................Passed
rstcheck.................................................................Passed
Validate GitHub Workflows................................................Passed
Validate GitHub Actions..............................(no files to check)Skipped
Validate Dependabot Config (v2)..........................................Passed
codespell................................................................Passed
executable-shell.........................................................Passed

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I have invited you to access my forked repo, clone it, check out the detailed-error-msg branch, run pre-commit in your environment and if it works then push the changes here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hi @sleepyStick!
Please help to merge this PR.
Does the tools\synchro.py migrate error message changes?

Copy link
Contributor

Choose a reason for hiding this comment

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

Sorry just seeing this, let me try this now :)

Copy link
Contributor

Choose a reason for hiding this comment

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

I just ran pre-commit on my local copy of your branch and synchro looked like this (which is what i expected)

synchro..................................................................Failed
- hook id: synchro
- files were modified by this hook

just pushed those changes :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sorry just seeing this, let me try this now :)

Hey, thanks for listening, you are a gem!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I just ran pre-commit on my local copy of your branch and synchro looked like this (which is what i expected)

synchro..................................................................Failed
- hook id: synchro
- files were modified by this hook

just pushed those changes :)

Oh, then something is wrong with my setup.

Original file line number Diff line number Diff line change
Expand Up @@ -161,7 +161,7 @@ def _password_digest(username: str, password: str) -> str:
if len(password) == 0:
raise ValueError("password can't be empty")
if not isinstance(username, str):
raise TypeError("username must be an instance of str")
raise TypeError(f"username must be an instance of str, not {type(username)}")

md5hash = hashlib.md5() # noqa: S324
data = f"{username}:mongo:{password}"
Expand Down
4 changes: 3 additions & 1 deletion pymongo/asynchronous/auth_oidc.py
Original file line number Diff line number Diff line change
Expand Up @@ -213,7 +213,9 @@ def _get_access_token(self) -> Optional[str]:
)
resp = cb.fetch(context)
if not isinstance(resp, OIDCCallbackResult):
raise ValueError("Callback result must be of type OIDCCallbackResult")
raise ValueError(
f"Callback result must be of type OIDCCallbackResult, not {type(resp)}"
)
self.refresh_token = resp.refresh_token
self.access_token = resp.access_token
self.token_gen_id += 1
Expand Down
12 changes: 9 additions & 3 deletions pymongo/asynchronous/client_session.py
Original file line number Diff line number Diff line change
Expand Up @@ -310,7 +310,9 @@ def __init__(
)
if max_commit_time_ms is not None:
if not isinstance(max_commit_time_ms, int):
raise TypeError("max_commit_time_ms must be an integer or None")
raise TypeError(
f"max_commit_time_ms must be an integer or None, not {type(max_commit_time_ms)}"
)

@property
def read_concern(self) -> Optional[ReadConcern]:
Expand Down Expand Up @@ -902,7 +904,9 @@ def advance_cluster_time(self, cluster_time: Mapping[str, Any]) -> None:
another `AsyncClientSession` instance.
"""
if not isinstance(cluster_time, _Mapping):
raise TypeError("cluster_time must be a subclass of collections.Mapping")
raise TypeError(
f"cluster_time must be a subclass of collections.Mapping, not {type(cluster_time)}"
)
if not isinstance(cluster_time.get("clusterTime"), Timestamp):
raise ValueError("Invalid cluster_time")
self._advance_cluster_time(cluster_time)
Expand All @@ -923,7 +927,9 @@ def advance_operation_time(self, operation_time: Timestamp) -> None:
another `AsyncClientSession` instance.
"""
if not isinstance(operation_time, Timestamp):
raise TypeError("operation_time must be an instance of bson.timestamp.Timestamp")
raise TypeError(
f"operation_time must be an instance of bson.timestamp.Timestamp, not {type(operation_time)}"
)
self._advance_operation_time(operation_time)

def _process_response(self, reply: Mapping[str, Any]) -> None:
Expand Down
10 changes: 5 additions & 5 deletions pymongo/asynchronous/collection.py
Original file line number Diff line number Diff line change
Expand Up @@ -228,7 +228,7 @@ def __init__(
read_concern or database.read_concern,
)
if not isinstance(name, str):
raise TypeError("name must be an instance of str")
raise TypeError(f"name must be an instance of str, not {type(name)}")
from pymongo.asynchronous.database import AsyncDatabase

if not isinstance(database, AsyncDatabase):
Expand Down Expand Up @@ -2475,7 +2475,7 @@ async def _drop_index(
name = helpers_shared._gen_index_name(index_or_name)

if not isinstance(name, str):
raise TypeError("index_or_name must be an instance of str or list")
raise TypeError(f"index_or_name must be an instance of str or list, not {type(name)}")

cmd = {"dropIndexes": self._name, "index": name}
cmd.update(kwargs)
Expand Down Expand Up @@ -3078,7 +3078,7 @@ async def rename(

"""
if not isinstance(new_name, str):
raise TypeError("new_name must be an instance of str")
raise TypeError(f"new_name must be an instance of str, not {type(new_name)}")

if not new_name or ".." in new_name:
raise InvalidName("collection names cannot be empty")
Expand Down Expand Up @@ -3148,7 +3148,7 @@ async def distinct(

"""
if not isinstance(key, str):
raise TypeError("key must be an instance of str")
raise TypeError(f"key must be an instance of str, not {type(key)}")
cmd = {"distinct": self._name, "key": key}
if filter is not None:
if "query" in kwargs:
Expand Down Expand Up @@ -3196,7 +3196,7 @@ async def _find_and_modify(
common.validate_is_mapping("filter", filter)
if not isinstance(return_document, bool):
raise ValueError(
"return_document must be ReturnDocument.BEFORE or ReturnDocument.AFTER"
f"return_document must be ReturnDocument.BEFORE or ReturnDocument.AFTER, not {type(return_document)}"
)
collation = validate_collation_or_none(kwargs.pop("collation", None))
cmd = {"findAndModify": self._name, "query": filter, "new": return_document}
Expand Down
6 changes: 4 additions & 2 deletions pymongo/asynchronous/command_cursor.py
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,9 @@ def __init__(
self.batch_size(batch_size)

if not isinstance(max_await_time_ms, int) and max_await_time_ms is not None:
raise TypeError("max_await_time_ms must be an integer or None")
raise TypeError(
f"max_await_time_ms must be an integer or None, not {type(max_await_time_ms)}"
)

def __del__(self) -> None:
self._die_no_lock()
Expand All @@ -115,7 +117,7 @@ def batch_size(self, batch_size: int) -> AsyncCommandCursor[_DocumentType]:
:param batch_size: The size of each batch of results requested.
"""
if not isinstance(batch_size, int):
raise TypeError("batch_size must be an integer")
raise TypeError(f"batch_size must be an integer, not {type(batch_size)}")
if batch_size < 0:
raise ValueError("batch_size must be >= 0")

Expand Down
Loading
Loading