Skip to content

Commit

Permalink
Code review feedback
Browse files Browse the repository at this point in the history
  • Loading branch information
jonathangreen committed Nov 19, 2024
1 parent 1ca1dce commit 484a7b0
Show file tree
Hide file tree
Showing 3 changed files with 4 additions and 15 deletions.
4 changes: 2 additions & 2 deletions src/palace/manager/opds/palace.py
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ class PalacePublicationMetadata(BaseOpdsModel, LoggerMixin):
# type of publication based on the type set, it would be nice to do some additional validation here
# and constrain this to PublicationTypes. Right now the Palace Bookshelf feed uses
# 'https://schema.org/EBook' (which is not a valid type) both because it starts with
# https:// (schema.org uses http://) and because it's its a Format, not a Type. Once we
# https:// (schema.org uses http://) and because its a Format, not a Type. Once we get
# this sorted out, we should add validation here. For now we just accept any string but
# log a warning if it's not a valid PublicationType.
type: str = Field(..., alias="@type")
Expand All @@ -86,6 +86,6 @@ class PalacePublicationMetadata(BaseOpdsModel, LoggerMixin):
@field_validator("type")
@classmethod
def warning_when_type_is_not_valid(cls, type_: str) -> str:
if type_ not in [t for t in PublicationTypes]:
if type_ not in list(PublicationTypes):
cls.logger().warning(f"@type '{type_}' is not a valid PublicationType.")
return type_
13 changes: 1 addition & 12 deletions src/palace/manager/opds/types/currency.py
Original file line number Diff line number Diff line change
Expand Up @@ -44,18 +44,7 @@ def __get_pydantic_core_schema__(
cls, _: type[Any], __: GetCoreSchemaHandler
) -> core_schema.CoreSchema:
"""
Return a Pydantic CoreSchema with the currency subset of the
[ISO4217](https://en.wikipedia.org/wiki/ISO_4217) format.
It excludes bonds testing codes and precious metals.
Args:
_: The source type.
__: The handler to get the CoreSchema.
Returns:
A Pydantic CoreSchema with the subset of the currency subset of the
[ISO4217](https://en.wikipedia.org/wiki/ISO_4217) format.
It excludes bonds testing codes and precious metals.
Return a Pydantic CoreSchema for validating this object.
"""
return core_schema.with_info_after_validator_function(
cls._validate,
Expand Down
2 changes: 1 addition & 1 deletion tests/manager/opds/types/test_language.py
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,7 @@ def test__eq__(self) -> None:
assert test_map != "baz"

# When comparing with a dictionary, the comparison will return True
# the dictionary would create an equivalent LanguageMap.
# if the dictionary would create an equivalent LanguageMap.
assert test_map == {"eng": "foo", "fra": "bar"}
assert test_map == {"fr": "bar", "en": "foo"}
assert test_map != {"en": "foo"}
Expand Down

0 comments on commit 484a7b0

Please sign in to comment.