-
Notifications
You must be signed in to change notification settings - Fork 7
Better match #52
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
Better match #52
Conversation
…ether to flatter, and whether to filter by type.
…ed if nothing is explicitly selected.
# Conflicts: # src/krrood/entity_query_language/entity.py
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This pull request refactors the entity query language to improve modularity by extracting quantifier and matching logic into dedicated modules. The refactoring moves quantifier functions (an, a, the) to a new quantify_entity.py module and matching functions (match, entity_matching, select, match_any, select_any) to a new match.py module, making the codebase more organized and easier to extend.
- Introduced new modules
quantify_entity.pyandmatch.pyto house previously scattered functionality - Enhanced the matching API with new
select(),match_any(), andselect_any()functions for more expressive queries - Updated type annotations and utilities including a new
is_iterable_type()function and refactoredSelectablebase class
Reviewed changes
Copilot reviewed 26 out of 26 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| src/krrood/entity_query_language/quantify_entity.py | New module containing quantifier functions (an, a, the) previously in entity.py |
| src/krrood/entity_query_language/match.py | New module containing all matching logic (Match, MatchEntity, Select classes and related functions) |
| src/krrood/entity_query_language/entity.py | Removed quantifier and match functions, streamlined to core entity logic |
| src/krrood/entity_query_language/symbolic.py | Added operand_value property, introduced Selectable base class, improved Literal initialization |
| src/krrood/entity_query_language/utils.py | Added is_iterable_type() utility function |
| test/test_eql/test_match.py | New test file for match functionality |
| test/test_eql/test_core/test_queries.py | Removed legacy match test (moved to dedicated test file) |
| test/test_* (multiple files) | Updated imports to use new quantify_entity module |
| examples/eql/match.md | Added documentation for select(), match_any(), and select_any() functions |
| examples/eql/* (multiple files) | Updated imports to use new quantify_entity module |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
LucaKro
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
well done with the refactor! just a few more things
| """ | ||
| try: | ||
| clazz = self.get_wrapped_class(clazz) | ||
| except ClassIsUnMappedInClassDiagram: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
very weird way of checking if the class already belings to the class diagram
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
how else would it be?
|
|
||
| def match_any( | ||
| type_: Union[Type[T], CanBehaveLikeAVariable[T], Any, None] = None, | ||
| ) -> Union[Type[T], CanBehaveLikeAVariable[T], Match[T]]: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
bind these unions to a type variable, otherwise this is hell to maintain
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tried but they remove the IDE hints unfortunately
removed is_universal_match and is_existential_match
This pull request refactors the entity query language implementation to improve modularity, clarity, and maintainability. The main changes include moving quantifier logic to a dedicated module, simplifying and reorganizing matching logic, introducing new selection and matching patterns in the documentation, and making several type and utility improvements. These changes collectively make the codebase easier to extend and understand, and provide clearer guidance in the documentation for users.
API and Codebase Refactoring
an,a,the) and related logic fromentity.pyinto a new modulequantify_entity.py, improving separation of concerns and modularity. Test imports were updated accordingly. [1] [2] [3] [4] [5] [6]Match,MatchEntity,match, andentity_matchingimplementations fromentity.py(now imported from a new module), streamlining the core entity logic.Documentation and Usability
examples/eql/match.mdwith new sections and examples forselect(),match_any(), andselect_any()to illustrate how to select and match inner objects and elements in collections. This helps users understand advanced query patterns.Type and Utility Improvements
is_iterable_type()utility function to check if a type is iterable, and improvedmake_list()usage for more robust type handling. [1] [2] [3]Selectablebase class and updatingCanBehaveLikeAVariableto inherit from it, clarifying responsibilities and improving extensibility.Minor Enhancements and Cleanups
operand_valueproperty toOperationResultfor easier access to operand values.These changes collectively modernize the query language implementation and make it easier for new contributors and users to understand and use advanced query features.