Merged
Conversation
Closed
This change refactors the model registry to use lazy loading. Instead of importing all model classes at startup, the registry now stores import paths as strings. Models are dynamically imported only when they are first accessed via registry.get(). This significantly reduces the initial import time and memory consumption of the apmodel package.
Member
Author
Todo
|
This commit resolves all type errors reported by pyrefly. The following changes were made: - tests/test_real_data.py: Adjusted assertions to correctly access model_extra attributes and check for None values for optional fields. - tests/test_registry.py: Corrected type hints for MINIMAL_PRELOADS and CustomObject.type to align with expected types. Imported Optional. - src/apmodel/_core/_initial/_rebuild.py: Updated type hint for models_to_rebuild to List[Type[ActivityPubModel]] and removed Nodeinfo related classes as they do not inherit ActivityPubModel. Also simplified the rebuild loop. - src/apmodel/_core/key.py: Added ValueError for unsupported key types to ensure 'wrapped' is always initialized. - src/apmodel/context.py: Removed __iter__ method from LDContext as it was inconsistent with BaseModel's __iter__. - src/apmodel/core/collection.py: Used typing.cast to explicitly cast return values of load functions in field validators. Imported cast. - src/apmodel/extra/litepub/emoji_react.py: Changed EmojiReact.content type to Optional[str] to match parent class Like. Imported Optional. - src/apmodel/_core/_initial/_registry_bootstrap.py: Added explicit type hint for TYPE_MAPPING to ensure compatibility with ModelRegistry. - src/apmodel/vocab/activity/question.py: Used typing.cast to explicitly cast return values of load functions in field validators. Imported cast. - src/apmodel/vocab/tombstone.py: Used typing.cast to explicitly cast return values of load functions in field validators. Imported cast.
for more information, see https://pre-commit.ci
This reverts commit 8647242.
for more information, see https://pre-commit.ci
Member
Author
|
Check It later, last correctly working version... |
items Updated Collection's first, current, last fields to explicitly include OrderedCollectionPage in their type hints, allowing Pydantic to correctly deserialize them as such. Refactored Collection and OrderedCollection to correctly handle ordered_items and items fields. Strings within these lists are now converted to Link objects, and dictionaries with a 'type' field are loaded as their respective models. Modified test_akkoma_replies to align with the corrected loading behavior of OrderedCollectionPage. Also updated various assert statements in test_real_data.py for better clarity and conciseness by replacing is not None with direct boolean evaluation where appropriate.
for more information, see https://pre-commit.ci
for more information, see https://pre-commit.ci
Member
Author
|
/gemini review |
Contributor
There was a problem hiding this comment.
Code Review
This is an impressive pull request that undertakes a major migration from dataclasses to Pydantic. The new implementation is much more robust, leveraging Pydantic's validation and serialization capabilities effectively. The addition of comprehensive documentation, a lazy-loading model registry, and tests against real-world data from various Fediverse platforms are all fantastic improvements.
My review focuses on a few areas for potential refinement:
- A correctness issue in cryptographic key encoding.
- Opportunities to improve maintainability in the pre-commit configuration and the custom JSON-LD context handling logic.
Overall, this is a high-quality contribution that significantly modernizes the library's core.
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
The serialization logic for the @context property was inconsistent. It would produce a string for a single context URL and a list for multiple URLs. This change ensures that @context is always serialized as a list, providing a predictable structure for consumers and simplifying client-side parsing. BREAKING CHANGE: The @context property is now always serialized as a list, even if it contains only a single item. Consumers that previously expected a string must now handle a list.
This commit introduces the ActorEndpoints model, integrating it into
the system's initial model rebuild process.
Extensive new test suites have been added to enhance coverage and
validate the functionality of several models, including:
- Actor and its various subtypes
- CryptographicKey, covering PEM and multibase key handling
- DataIntegrityProof, including datetime conversions and
serialization
- Multikey, with multibase encoding/decoding for public and private
keys
- Question, validating fields like oneOf, anyOf, and closed
- Tombstone, verifying formerType and deleted fields
- ActivityPubModel, testing base class features and context
aggregation
- Core key utility functions for multibase encoding/decoding
Additionally, the datetime serialization in the Question model has
been updated to consistently use the 'Z' suffix for UTC offsets,
ensuring standard compliance and predictability. This also includes
validation for the recently adjusted @context serialization behavior.
for more information, see https://pre-commit.ci
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
resolve #9