feat: well-defined type comparison semantics #846
Conversation
|
@coderabbitai review |
WalkthroughThe changes introduce enhancements to the type system documentation and validation logic within the codebase. Key updates include the addition of type comparison rules, support for enumerations, and clarifications on subtype requirements in injection mechanisms. The validation process has been improved with new structures for error handling and subtype checks. Additionally, comprehensive tests for type comparisons have been implemented to ensure correct behavior across various data types. Changes
Suggested reviewers
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Actionable comments posted: 2
Outside diff range and nitpick comments (10)
docs/metatype.dev/docs/reference/types/comparison.mdx (7)
14-14: Consider adding a comma for clarity.To improve readability, consider adding a comma after "In this section" as suggested by the static analysis hint.
-In this section `A` and `B` denote types, `U` and `V` denote lists of types. +In this section, `A` and `B` denote types, `U` and `V` denote lists of types.Tools
LanguageTool
[uncategorized] ~14-~14: Possible missing comma found.
Context: ...ections#parent). ## Semantics In this sectionAandBdenote types,UandVd...(AI_HYDRA_LEO_MISSING_COMMA)
25-26: Rephrase for clarity and grammatical correctness.To improve clarity and address the grammatical issues flagged by the static analysis hints, consider rephrasing the sentence as follows:
-If `A` is a subtype of `B` and `B` is a scalar type then - `A` is a scalar type of a union type in which any variant is a subtype of `B`. +If `A` is a subtype of `B`, and `B` is a scalar type, then + `A` is either a scalar type or a union type in which any variant is a subtype of `B`.Tools
LanguageTool
[uncategorized] ~25-~25: Possible missing comma found.
Context: ...is a subtype ofBandBis a scalar type thenAis a scalar type of a unio...(AI_HYDRA_LEO_MISSING_COMMA)
[grammar] ~26-~26: If ‘type’ is a classification term, ‘a’ is not necessary. Use “type of”. (The phrases ‘kind of’ and ‘sort of’ are informal if they mean ‘to some extent’.)
Context: ... a scalar type thenAis a scalar type of a union type in which any variant is a su...(KIND_OF_A)
30-30: Add missing article for grammatical correctness.To address the grammatical issue flagged by the static analysis hint, add the missing article "a" as follows:
-Then `A` is subtype of `B` if and only if the following conditions hold: +Then `A` is a subtype of `B` if and only if the following conditions hold:Tools
LanguageTool
[uncategorized] ~30-~30: You might be missing the article “a” here.
Context: ...Bare scalar types. ThenAis subtype ofBif and only if the following con...(AI_EN_LECTOR_MISSING_DETERMINER_A)
60-60: Add missing article for grammatical correctness.To address the grammatical issue flagged by the static analysis hint, add the missing article "a" as follows:
-`A` is a subtype of `t.optional(B)` if and only if `A` is subtype of `B`. +`A` is a subtype of `t.optional(B)` if and only if `A` is a subtype of `B`.Tools
LanguageTool
[uncategorized] ~60-~60: You might be missing the article “a” here.
Context: ...ft.optional(B)if and only ifAis subtype ofB. 2.t.optional(A)is a subtyp...(AI_EN_LECTOR_MISSING_DETERMINER_A)
69-69: Fix the spelling of "vice versa".To address the grammatical issue flagged by the static analysis hint, remove the hyphen from "vice-versa" as follows:
-If `A` is a list then `B` is a list and vice-versa. +If `A` is a list then `B` is a list and vice versa. -If `A` is an object type then `B` is an object type, and vice-versa. +If `A` is an object type then `B` is an object type, and vice versa.Also applies to: 87-87
Tools
LanguageTool
[grammar] ~69-~69: The expression “vice versa” is spelled without hyphens.
Context: ...IfAis a list thenBis a list and vice-versa. 2. LetXa list type whose item typ...(VICE_VERSA)
87-87: Consider adding commas for clarity.To improve readability, consider adding commas as suggested by the static analysis hints:
-If `A` is an object type then `B` is an object type, and vice versa. +If `A` is an object type, then `B` is an object type, and vice versa. -If `A` is a type and `V` a list of types then +If `A` is a type, and `V` a list of types, then -If `A` is a type and `V` a list of types then +If `A` is a type, and `V` a list of types, thenAlso applies to: 103-103, 107-107
Tools
LanguageTool
[uncategorized] ~87-~87: Possible missing comma found.
Context: ... subtype ofB. IfAis an object type thenBis an object type, and vice-ve...(AI_HYDRA_LEO_MISSING_COMMA)
[grammar] ~87-~87: The expression “vice versa” is spelled without hyphens.
Context: ...ct type thenBis an object type, and vice-versa. 2. IfAandBare object types, t...(VICE_VERSA)
51-51: Consider a shorter alternative for "exactly the same".While "exactly the same" is a common phrase, using a shorter alternative can improve conciseness. Consider using "identical" or simply "the same" to convey the same meaning more concisely.
-The patterns are required to be exactly the same +The patterns are required to be identicalTools
LanguageTool
[style] ~51-~51: ‘exactly the same’ might be wordy. Consider a shorter alternative.
Context: ...[^pat]: The patterns are required to be exactly the same because we currently do not hav...(EN_WORDINESS_PREMIUM_EXACTLY_THE_SAME)
tests/schema_validation/type_comparison.py (2)
76-104: Good test coverage for file type comparison with some follow-up items!The test cases cover a comprehensive set of scenarios for file type comparison, including min, max, and allow constraints. The test cases are correctly structured to check both successful type matches and expected failures.
However, there are two items that require follow-up:
- The TODO comment at line 93 indicates that support for wildcards in the allow constraint is missing. This should be addressed in the future to enhance the functionality of the file type comparison.
- The FIXME comment at line 95 indicates that there is a potential issue with the test case "file_fail_1" where it is expected to fail but it doesn't. This should be investigated and fixed to ensure the correctness of the test suite.
Do you want me to open GitHub issues to track these follow-up items?
185-191: Good test coverage for enum type comparison with a follow-up item!The test cases cover scenarios where enum subtypes are compared with enum supertypes and non-enum types. The test cases are correctly structured to check both successful type matches and expected failures.
However, there is a TODO comment at line 188 indicating that a test case for comparing float enum with integer enum is missing. This should be added in the future to enhance the coverage of the enum type comparison test suite.
Do you want me to open a GitHub issue to track this follow-up item?
docs/metatype.dev/docs/reference/types/index.mdx (1)
112-113: Improve readability by adding a comma.Consider adding a comma after the introductory phrase "In this case" for better readability:
-In this case the only valid values for the type are the ones that are listed in the enumeration. +In this case, the only valid values for the type are the ones that are listed in the enumeration.Tools
LanguageTool
[uncategorized] ~112-~112: Loose punctuation mark.
Context: ...DKTabs> ### Enumerationst.integer(),t.float(),t.string(), `t.struct(.....(UNLIKELY_OPENING_PUNCTUATION)
[formatting] ~112-~112: Consider inserting a comma after an introductory phrase for better readability.
Context: ...` can have a list of enumerated values. In this case the only valid values for the type are ...(IN_THAT_CASE_COMMA)
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files ignored due to path filters (2)
tests/e2e/typegraph/__snapshots__/validator_test.ts.snapis excluded by!**/*.snaptests/schema_validation/__snapshots__/type_comparison_test.ts.snapis excluded by!**/*.snap
Files selected for processing (9)
- docs/metatype.dev/docs/reference/types/comparison.mdx (1 hunks)
- docs/metatype.dev/docs/reference/types/index.mdx (1 hunks)
- docs/metatype.dev/docs/reference/types/injections.mdx (1 hunks)
- src/common/src/typegraph/types.rs (1 hunks)
- src/common/src/typegraph/validator/mod.rs (2 hunks)
- src/common/src/typegraph/validator/types.rs (3 hunks)
- src/common/src/typegraph/visitor.rs (7 hunks)
- tests/schema_validation/type_comparison.py (1 hunks)
- tests/schema_validation/type_comparison_test.ts (1 hunks)
Additional context used
LanguageTool
docs/metatype.dev/docs/reference/types/comparison.mdx
[uncategorized] ~14-~14: Possible missing comma found.
Context: ...ections#parent). ## Semantics In this sectionAandBdenote types,UandVd...(AI_HYDRA_LEO_MISSING_COMMA)
[uncategorized] ~25-~25: Possible missing comma found.
Context: ...is a subtype ofBandBis a scalar type thenAis a scalar type of a unio...(AI_HYDRA_LEO_MISSING_COMMA)
[grammar] ~26-~26: If ‘type’ is a classification term, ‘a’ is not necessary. Use “type of”. (The phrases ‘kind of’ and ‘sort of’ are informal if they mean ‘to some extent’.)
Context: ... a scalar type thenAis a scalar type of a union type in which any variant is a su...(KIND_OF_A)
[uncategorized] ~30-~30: You might be missing the article “a” here.
Context: ...Bare scalar types. ThenAis subtype ofBif and only if the following con...(AI_EN_LECTOR_MISSING_DETERMINER_A)
[style] ~51-~51: ‘exactly the same’ might be wordy. Consider a shorter alternative.
Context: ...[^pat]: The patterns are required to be exactly the same because we currently do not hav...(EN_WORDINESS_PREMIUM_EXACTLY_THE_SAME)
[uncategorized] ~60-~60: You might be missing the article “a” here.
Context: ...ft.optional(B)if and only ifAis subtype ofB. 2.t.optional(A)is a subtyp...(AI_EN_LECTOR_MISSING_DETERMINER_A)
[grammar] ~69-~69: The expression “vice versa” is spelled without hyphens.
Context: ...IfAis a list thenBis a list and vice-versa. 2. LetXa list type whose item typ...(VICE_VERSA)
[uncategorized] ~87-~87: Possible missing comma found.
Context: ... subtype ofB. IfAis an object type thenBis an object type, and vice-ve...(AI_HYDRA_LEO_MISSING_COMMA)
[grammar] ~87-~87: The expression “vice versa” is spelled without hyphens.
Context: ...ct type thenBis an object type, and vice-versa. 2. IfAandBare object types, t...(VICE_VERSA)
[uncategorized] ~103-~103: Possible missing comma found.
Context: ... 1. IfAis a type andVa list of types thenAis a subtype of `t.union(V...(AI_HYDRA_LEO_MISSING_COMMA)
[uncategorized] ~107-~107: Possible missing comma found.
Context: ... 2. IfAis a type andVa list of types thenAis a subtype of `t.either(...(AI_HYDRA_LEO_MISSING_COMMA)
docs/metatype.dev/docs/reference/types/index.mdx
[uncategorized] ~112-~112: Loose punctuation mark.
Context: ...DKTabs> ### Enumerationst.integer(),t.float(),t.string(), `t.struct(.....(UNLIKELY_OPENING_PUNCTUATION)
[formatting] ~112-~112: Consider inserting a comma after an introductory phrase for better readability.
Context: ...` can have a list of enumerated values. In this case the only valid values for the type are ...(IN_THAT_CASE_COMMA)
Additional comments not posted (38)
docs/metatype.dev/docs/reference/types/injections.mdx (1)
18-18: Documentation update looks good!The added requirement clarifies the type compatibility constraint for the parent injection mechanism. This is an important point to ensure type safety during dependency resolution.
The change improves the documentation by making it more complete and easier to understand.
src/common/src/typegraph/validator/mod.rs (4)
12-13: LGTM!The additional imports are necessary for the subsequent changes to implement the new layer of functionality for type graph validation.
19-19: LGTM!The inclusion of the
Layerparameter in thetraverse_typesmethod call indicates a more structured approach to visiting nodes within the type graph. This change aligns with the overall objective of improving the type graph validation process.
38-60: LGTM!The introduction of the
Layerstruct and its implementation of theVisitLayertrait for theValidatorenhances the structure and error handling capabilities of the type graph validation process. Thevisitmethod's logic for iterating over child nodes and collecting errors provides a more granular approach to handling validation errors during the traversal.
61-61: LGTM!The addition of the empty line improves the readability and organization of the code by visually separating the
Layerstruct implementation from the subsequent code.docs/metatype.dev/docs/reference/types/comparison.mdx (4)
1-11: Excellent introduction to subtyping and type comparison!The introduction provides a clear and concise explanation of subtyping, using the analogy of the "is subset of" relation to help readers understand the concept. The mention of the relevance to "from parent" injections provides good context for the importance of type comparison.
12-117: Comprehensive and well-structured semantics section!The semantics section provides detailed rules and conditions for type comparison in each type category. The use of tables to present the conditions for type constraints enhances readability. The explanations are clear and thorough.
Tools
LanguageTool
[uncategorized] ~14-~14: Possible missing comma found.
Context: ...ections#parent). ## Semantics In this sectionAandBdenote types,UandVd...(AI_HYDRA_LEO_MISSING_COMMA)
[uncategorized] ~25-~25: Possible missing comma found.
Context: ...is a subtype ofBandBis a scalar type thenAis a scalar type of a unio...(AI_HYDRA_LEO_MISSING_COMMA)
[grammar] ~26-~26: If ‘type’ is a classification term, ‘a’ is not necessary. Use “type of”. (The phrases ‘kind of’ and ‘sort of’ are informal if they mean ‘to some extent’.)
Context: ... a scalar type thenAis a scalar type of a union type in which any variant is a su...(KIND_OF_A)
[uncategorized] ~30-~30: You might be missing the article “a” here.
Context: ...Bare scalar types. ThenAis subtype ofBif and only if the following con...(AI_EN_LECTOR_MISSING_DETERMINER_A)
[style] ~51-~51: ‘exactly the same’ might be wordy. Consider a shorter alternative.
Context: ...[^pat]: The patterns are required to be exactly the same because we currently do not hav...(EN_WORDINESS_PREMIUM_EXACTLY_THE_SAME)
[uncategorized] ~60-~60: You might be missing the article “a” here.
Context: ...ft.optional(B)if and only ifAis subtype ofB. 2.t.optional(A)is a subtyp...(AI_EN_LECTOR_MISSING_DETERMINER_A)
[grammar] ~69-~69: The expression “vice versa” is spelled without hyphens.
Context: ...IfAis a list thenBis a list and vice-versa. 2. LetXa list type whose item typ...(VICE_VERSA)
[uncategorized] ~87-~87: Possible missing comma found.
Context: ... subtype ofB. IfAis an object type thenBis an object type, and vice-ve...(AI_HYDRA_LEO_MISSING_COMMA)
[grammar] ~87-~87: The expression “vice versa” is spelled without hyphens.
Context: ...ct type thenBis an object type, and vice-versa. 2. IfAandBare object types, t...(VICE_VERSA)
[uncategorized] ~103-~103: Possible missing comma found.
Context: ... 1. IfAis a type andVa list of types thenAis a subtype of `t.union(V...(AI_HYDRA_LEO_MISSING_COMMA)
[uncategorized] ~107-~107: Possible missing comma found.
Context: ... 2. IfAis a type andVa list of types thenAis a subtype of `t.either(...(AI_HYDRA_LEO_MISSING_COMMA)
117-120: Good reference to the type comparison test suite!Providing a link to the type comparison test suite is helpful for readers who want to see practical examples of type comparison. The section is concise and to the point.
1-120: Excellent document structure and comprehensive content!The document is well-structured, with clear sections for introduction, semantics, and examples. The content is comprehensive, covering type comparison rules and semantics for various type categories. The explanations are clear and thorough, making effective use of tables and code snippets.
This document serves as a valuable reference for understanding type comparison in the metatype system and provides a solid foundation for developers working with type injections and validation.
Tools
LanguageTool
[uncategorized] ~14-~14: Possible missing comma found.
Context: ...ections#parent). ## Semantics In this sectionAandBdenote types,UandVd...(AI_HYDRA_LEO_MISSING_COMMA)
[uncategorized] ~25-~25: Possible missing comma found.
Context: ...is a subtype ofBandBis a scalar type thenAis a scalar type of a unio...(AI_HYDRA_LEO_MISSING_COMMA)
[grammar] ~26-~26: If ‘type’ is a classification term, ‘a’ is not necessary. Use “type of”. (The phrases ‘kind of’ and ‘sort of’ are informal if they mean ‘to some extent’.)
Context: ... a scalar type thenAis a scalar type of a union type in which any variant is a su...(KIND_OF_A)
[uncategorized] ~30-~30: You might be missing the article “a” here.
Context: ...Bare scalar types. ThenAis subtype ofBif and only if the following con...(AI_EN_LECTOR_MISSING_DETERMINER_A)
[style] ~51-~51: ‘exactly the same’ might be wordy. Consider a shorter alternative.
Context: ...[^pat]: The patterns are required to be exactly the same because we currently do not hav...(EN_WORDINESS_PREMIUM_EXACTLY_THE_SAME)
[uncategorized] ~60-~60: You might be missing the article “a” here.
Context: ...ft.optional(B)if and only ifAis subtype ofB. 2.t.optional(A)is a subtyp...(AI_EN_LECTOR_MISSING_DETERMINER_A)
[grammar] ~69-~69: The expression “vice versa” is spelled without hyphens.
Context: ...IfAis a list thenBis a list and vice-versa. 2. LetXa list type whose item typ...(VICE_VERSA)
[uncategorized] ~87-~87: Possible missing comma found.
Context: ... subtype ofB. IfAis an object type thenBis an object type, and vice-ve...(AI_HYDRA_LEO_MISSING_COMMA)
[grammar] ~87-~87: The expression “vice versa” is spelled without hyphens.
Context: ...ct type thenBis an object type, and vice-versa. 2. IfAandBare object types, t...(VICE_VERSA)
[uncategorized] ~103-~103: Possible missing comma found.
Context: ... 1. IfAis a type andVa list of types thenAis a subtype of `t.union(V...(AI_HYDRA_LEO_MISSING_COMMA)
[uncategorized] ~107-~107: Possible missing comma found.
Context: ... 2. IfAis a type andVa list of types thenAis a subtype of `t.either(...(AI_HYDRA_LEO_MISSING_COMMA)
src/common/src/typegraph/types.rs (1)
316-316: LGTM! The label change improves clarity and precision.The change from "number" to "float" for the
Floatvariant in thetype_namemethod provides a more accurate and specific representation of the type. This enhances code readability and maintainability by avoiding potential confusion between generic numeric types and floating-point types. The change aligns with the principle of using clear and precise naming conventions in the codebase.tests/schema_validation/type_comparison.py (9)
26-26: LGTM!The test case is correctly defined and should pass as a boolean type is always compatible with itself.
28-41: Great test coverage for integer type comparison!The test cases cover a comprehensive set of scenarios for integer type comparison, including min, max, and multiple_of constraints. The test cases are correctly structured to check both successful type matches and expected failures.
43-56: Great test coverage for float type comparison!The test cases cover a comprehensive set of scenarios for float type comparison, including min, max, and multiple_of constraints. The test cases are correctly structured to check both successful type matches and expected failures.
58-65: Good test coverage for number type comparison!The test cases cover scenarios where integer subtypes are compared with float supertypes and vice versa. The test cases are correctly structured to check both successful type matches and expected failures.
67-74: Great test coverage for string type comparison!The test cases cover a comprehensive set of scenarios for string type comparison, including min and max constraints. The test cases are correctly structured to check both successful type matches and expected failures.
106-113: Good test coverage for optional type comparison!The test cases cover scenarios where optional subtypes are compared with optional supertypes and non-optional types. The test cases are correctly structured to check both successful type matches and expected failures.
115-120: Good test coverage for list type comparison!The test cases cover scenarios where list subtypes are compared with list supertypes and non-list types. The test cases are correctly structured to check both successful type matches and expected failures.
122-154: Good test coverage for struct type comparison!The test cases cover scenarios where struct subtypes are compared with struct supertypes and struct types with different field names and types. The test cases are correctly structured to check both successful type matches and expected failures.
156-183: Excellent test coverage for union type comparison!The test cases cover a wide range of scenarios where union subtypes are compared with union supertypes, either types, and non-union types. The test cases are correctly structured to check both successful type matches and expected failures, ensuring the correctness of the union type comparison logic.
src/common/src/typegraph/visitor.rs (8)
9-11: LGTM!The
DefaultLayerstruct is a valid default implementation for theVisitLayertrait. DerivingCloneis necessary to satisfy the trait requirements.
14-21: LGTM!The
VisitLayertrait is a well-designed abstraction for customizable traversal layers. Thevisitmethod signature allows for flexible traversal strategies, and the associated types provide extensibility in the visitor and context used.
23-37: LGTM!The
DefaultLayerimplementation ofVisitLayerprovides a sensible default behavior for visiting child nodes. It allows for extensibility by serving as a baseline that can be overridden by custom layers.
39-46: LGTM!The
visit_childfunction is a useful abstraction that simplifies visiting child nodes. It encapsulates the details of calling thevisit_childmethod on theTypegraphTraversalinstance, making it easier for custom layers to visit child nodes.
50-58: LGTM!The modifications to the
traverse_typesmethod enhance the flexibility and extensibility of the type traversal mechanism. The addition of thelayerparameter allows for customizable traversal layers, and theSizedtrait bound onVensures that the visitor type can be used as a value.
82-93: LGTM!The modifications to the
TypegraphTraversalstruct make it more flexible and extensible. The addition of thelayerfield allows the traversal to utilize different traversal layers, and theSizedtrait bound onVensures that the visitor type can be used as a value within the traversal.
200-211: LGTM!The modification to the
visit_objectmethod to use the newvisit_childrenmethod improves the code structure and readability. Mapping the object properties toChildNodeinstances and passing them tovisit_childrenaligns with the new traversal layer abstraction and simplifies the process of visiting child nodes.
304-314: LGTM!The addition of the
visit_childrenmethod to theTypegraphTraversalstruct is a great improvement. It encapsulates the logic for visiting multiple child nodes, abstracting away the details of iteration and delegation to the traversal layer. This method promotes code reuse and enhances the readability of the traversal code.docs/metatype.dev/docs/reference/types/index.mdx (1)
110-142: LGTM!The new section on enumerations is a great addition to the type system documentation. The examples clearly demonstrate how to use the
.enum()method with various types, and thet.enum(array)alias provides a convenient shorthand for defining string enumerations.The changes enhance the functionality and usability of the type system by allowing developers to define constrained sets of values for their types.
Tools
LanguageTool
[uncategorized] ~112-~112: Loose punctuation mark.
Context: ...DKTabs> ### Enumerationst.integer(),t.float(),t.string(), `t.struct(.....(UNLIKELY_OPENING_PUNCTUATION)
[formatting] ~112-~112: Consider inserting a comma after an introductory phrase for better readability.
Context: ...` can have a list of enumerated values. In this case the only valid values for the type are ...(IN_THAT_CASE_COMMA)
src/common/src/typegraph/validator/types.rs (10)
27-33: LGTM!The new
push_nestedmethod looks good. It allows for nesting of error messages, which can be useful for providing more structured and hierarchical error reporting.
36-37: Great addition!The inclusion of the generic type parameter
Twith a default value ofSelfmakes theEnsureSubtypeOftrait more flexible. It allows for cross-type comparisons, such as comparing anIntegerTypeDatawith aFloatTypeData, which can be very useful in certain scenarios.
41-64: Nice refactoring!The use of pattern matching instead of nested
if letstatements improves the readability and maintainability of the code. The logic remains the same, ensuring that the minimum value of the subtype is not lower than the minimum value of the supertype. The error messages are clear and informative.
67-90: Nice refactoring!The use of pattern matching instead of nested
if letstatements improves the readability and maintainability of the code. The logic remains the same, ensuring that the maximum value of the subtype is not higher than the maximum value of the supertype. The error messages are clear and informative.
93-116: Nice refactoring!The use of pattern matching instead of nested
if letstatements improves the readability and maintainability of the code. The logic remains the same, ensuring that themultiple_ofvalue of the subtype is a multiple of themultiple_ofvalue of the supertype. The error messages are clear and informative.
119-141: LGTM!The implementation of
EnsureSubtypeOfforIntegerTypeDatalooks good. It properly checks the subtype constraints forminimum,maximum,exclusive_minimum,exclusive_maximum, andmultiple_ofvalues using the respective helper functions. This ensures that theIntegerTypeDatasubtype is compatible with its supertype.
144-166: LGTM!The implementation of
EnsureSubtypeOfforFloatTypeDatalooks good. It properly checks the subtype constraints forminimum,maximum,exclusive_minimum,exclusive_maximum, andmultiple_ofvalues using the respective helper functions. This ensures that theFloatTypeDatasubtype is compatible with its supertype.
168-206: Great implementation!The implementation of
EnsureSubtypeOfforIntegerTypeDatawithFloatTypeDataas the supertype is a nice addition. It allows for cross-type comparison, ensuring that theminimum,maximum,exclusive_minimum,exclusive_maximum, andmultiple_ofvalues of theIntegerTypeDatasubtype are compatible with the corresponding values of theFloatTypeDatasupertype. The conversion ofIntegerTypeDatafields tof64is handled correctly.
209-245: LGTM!The implementation of
EnsureSubtypeOfforStringTypeDatalooks good. It properly checks the subtype constraints formin_length,max_length,pattern, andformatfields. Themin_lengthandmax_lengthvalues are verified using the respective helper functions, while thepatternandformatfields are checked for exact equality if they are defined on both the subtype and supertype. The error messages are clear and informative.
248-269: LGTM!The implementation of `
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #846 +/- ##
==========================================
- Coverage 76.43% 76.21% -0.23%
==========================================
Files 144 144
Lines 16705 16705
Branches 1646 1642 -4
==========================================
- Hits 12769 12732 -37
- Misses 3917 3954 +37
Partials 19 19 ☔ View full report in Codecov by Sentry. |
Solve MET-655
EnsureSubtypeOftrait)Migration notes
No change is needed.
...
Summary by CodeRabbit
Release Notes
New Features
Bug Fixes
Documentation
Tests