added method for caching the PK of a Model #252
Conversation
Reviewer's Guide by SourceryThis pull request introduces functionality for defining and retrieving primary keys in Model classes, fixes a bug related to JSON encoding of lists with optional submodels, and increments the version number. Updated class diagram for ModelclassDiagram
class Model {
+get_primary_keys()
+get_primary_key_fields()
+get_primary_key_values()
}
Model : +get_primary_keys() - Added method
Model : +get_primary_key_fields() - Added method
Model : +get_primary_key_values() - Added method
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey @phenobarbital - I've reviewed your changes - here's some feedback:
Overall Comments:
- Consider adding a test case that verifies the behavior of
get_primary_key_fieldswhen a model has multiple primary keys. - It might be worth adding a short description of the new methods to the class docstring.
Here's what I looked at during the review
- 🟢 General issues: all looks good
- 🟢 Security: all looks good
- 🟢 Testing: all looks good
- 🟡 Complexity: 1 issue found
- 🟢 Documentation: all looks good
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| dc.modelName = dc.__name__ | ||
|
|
||
| # Override __setattr__ method | ||
| setattr(dc, "__setattr__", _dc_method_setattr_) |
There was a problem hiding this comment.
issue (complexity): Consider extracting the primary key handling logic into dedicated helper functions or class methods to reduce nesting and complexity within the __new__ method, improving readability and maintainability of the metaclass logic by separating concerns and reducing inline definitions, while preserving functionality related to primary keys.
Extract the primary key handling logic from the __new__ method into dedicated helper functions (or even class methods) to reduce nesting and inline complexity. For example, you can define the primary key extractor functions outside of __new__ so that they’re defined once rather than redefined every time a new model is built. This change keeps functionality intact while making the metaclass logic easier to read and maintain.
One possible approach is as follows:
-
Move inline functions out of
__new__:Define helper functions for retrieving primary keys and their fields at the class level:
class ModelMeta(type): ... @classmethod def get_primary_keys(cls): return cls.__primary_keys__ @classmethod def get_primary_key_fields(cls): """Return a dictionary of primary key fields with their types.""" return {name: cls.__columns__[name] for name in cls.__primary_keys__} def get_primary_key_values(self): """Get primary key values for this instance as a dictionary.""" return {key: getattr(self, key) for key in self.__primary_keys__}
-
Assign these methods after class creation instead of defining them inline:
In your
__new__method, assign the prepared functions instead of inline function definitions:def __new__(cls, name, bases, attrs, **kwargs): # noqa ... # After selecting and creating dc dc.__primary_keys__ = primary_keys # Assign the primary key helper methods directly dc.get_primary_keys = classmethod(ModelMeta.get_primary_keys) dc.get_primary_key_fields = classmethod(ModelMeta.get_primary_key_fields) dc.get_primary_key_values = ModelMeta.get_primary_key_values # instance method return dc
This refactoring separates concerns, reduces nested inline definitions, and preserves the functionality of handling primary keys.
Summary by Sourcery
Adds functionality to cache and retrieve primary key information for Model classes, including the ability to get primary key fields and values. Also, fixes an issue with JSON encoding of lists containing submodels when using Optional types, and increments the version number.
Enhancements: