Skip to content
This repository was archived by the owner on Feb 6, 2025. It is now read-only.

Conversation

@amartyashankha
Copy link
Collaborator

No description provided.

Copy link
Contributor

@obi1kenobi obi1kenobi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Love the clarifications, renamings, and additional documentation! Minor comments in a few places, and I really like where this is headed.



def extract_location_to_optional_from_ir_blocks(ir_blocks):
def extract_location_to_optional(ir_blocks):
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

extract_location_to_optional still seems kinda vague and not in line with the root-related renaming we did. How about something like extract_optional_location_root_info or something to that effect?

new_output_fields = {}
for output_name, expression in six.iteritems(output_block.fields):
if isinstance(expression, OutputContextField):
# An OutputContextField without a TernaryConditional should not be within
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see how we've established that the expression is an OutputContextField on the line above. I don't see where we've established that it is without a TernaryConditional though, and I'm confused.

.format(expression.location, present_locations))
new_output_fields[output_name] = expression
elif isinstance(expression, FoldedOutputContextField):
# A FoldedOutputContextField without a TernaryConditional should not be within
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same concern about TernaryConditional as above

@obi1kenobi obi1kenobi merged commit 8410438 into kensho-technologies:optional_traversal May 14, 2018
obi1kenobi added a commit that referenced this pull request May 21, 2018
* Allowing traversal inside `@optional` scope (#66)

* Added Gremlin support for traversing inside @optional.

* Added Gremlin support for traversing inside @optional.

* Starting optional traversal.

* Allowing optional traversal.

* Allowing optional traversal.

* Fixed isort errors.

* Fixed pylint errors.

* Temporarily disabled pylint.

* Fixed pylint errors.

* Simplified Traverse.get_field_name.

* Added test case

* Disallowed folds inside optional

* Added tests for conflicts with directives within @optional.

* Added test case for deep optional traverse after a filter.

* Fixed lint erroe

* Changed 'fold_last_level' to 'fold_innermost_scope', and wrote explicit check for 'fold_innermost_scope' in context at the end of a fold.

* Added test cases for traversing inside optional.

* Updated compiler_frontend documentation

* Fixed lint errors.

* Added docstrings. Reverted FoldedOutputContextField. Simplified emit_match.

* Changed function names.

* Fixed small errors.

* Fixed pylint error

* Allowing traversal inside `@optional` scope (#66)

* Added Gremlin support for traversing inside @optional.

* Added Gremlin support for traversing inside @optional.

* Starting optional traversal.

* Allowing optional traversal.

* Allowing optional traversal.

* Fixed isort errors.

* Fixed pylint errors.

* Temporarily disabled pylint.

* Fixed pylint errors.

* Simplified Traverse.get_field_name.

* Added test case

* Disallowed folds inside optional

* Added tests for conflicts with directives within @optional.

* Added test case for deep optional traverse after a filter.

* Fixed lint erroe

* Changed 'fold_last_level' to 'fold_innermost_scope', and wrote explicit check for 'fold_innermost_scope' in context at the end of a fold.

* Added test cases for traversing inside optional.

* Updated compiler_frontend documentation

* Fixed lint errors.

* Added docstrings. Reverted FoldedOutputContextField. Simplified emit_match.

* Changed function names.

* Fixed small errors.

* Fixed pylint error

* fixing imports in branch

* Removed duplicate functions

* Added documentation

* Removed Note/Notice

* Clarified optional traversal

* Fixed documentation.

* Spell checked.

* Optional traversal fixes (#69)

* Added Gremlin support for traversing inside @optional.

* Added Gremlin support for traversing inside @optional.

* Starting optional traversal.

* Allowing optional traversal.

* Allowing optional traversal.

* Fixed isort errors.

* Fixed pylint errors.

* Temporarily disabled pylint.

* Fixed pylint errors.

* Simplified Traverse.get_field_name.

* Added test case

* Disallowed folds inside optional

* Added tests for conflicts with directives within @optional.

* Added test case for deep optional traverse after a filter.

* Fixed lint erroe

* Changed 'fold_last_level' to 'fold_innermost_scope', and wrote explicit check for 'fold_innermost_scope' in context at the end of a fold.

* Added test cases for traversing inside optional.

* Updated compiler_frontend documentation

* Fixed lint errors.

* Fixed multiple concurrent filters for same location issue. Added test case for the same.

* Added test case for query involving optionals with and without a traversal.

* Added docstrings. Reverted FoldedOutputContextField. Simplified emit_match.

* Changed function names.

* Fixed multiple concurrent filters for same location issue. Added test case for the same.

* Added test case for query involving optionals with and without a traversal.

* Fixed small errors.

* Fixed multiple concurrent filters for same location issue. Added test case for the same.

* Added test case for query involving optionals with and without a traversal.

* Fixed multiple concurrent filters for same location issue. Added test case for the same.

* Fixed pylint error

* Fixed multiple concurrent filters for same location issue. Added test case for the same.

* Added test case for query involving optionals with and without a traversal.

* Fixed multiple concurrent filters for same location issue. Added test case for the same.

* Fixed multiple concurrent filters for same location issue. Added test case for the same.

* Fixed multiple concurrent filters for same location issue. Added test case for the same.

* Added test case for query involving optionals with and without a traversal.

* Fixed merge conflict.

* Fixed lint error

* Fixed errors from previous PR(#66)

* Fixed errors

* Fixed flake8 errors

* Making nested function private

* Fixed docstring

* Started ficx for apllowing tags inside optional traverse.

* Making nested function private underscored

* Started ficx for apllowing tags inside optional traverse.

* Tags allowed inside optional traversal

* Added test cases for tags inside optional traversal

* Allowing Recursion within optional traversal. Gremlin support might need an update in blocks.Recurse

* Fixed lint error

* Fixed lint error

* Added simple test-case for Recursion within optional traversal.

* Fixed CopySplit indentation

* Fixed CopySplit indentation

* Allowing Brached traversals within optional

* Fixed IR generation test cases to inclue EndOptional blocks.

* Removed dead code. Fixed lint errors

* Fixed flake8 error.

* Fixed pytlint errors.

* Fixed imports

* Extracted tag update visitor functions.

* Addresed comments

* Fixed iteritems error

* Decomposed function collect_filters_to_first_location_instance

* Decomposed function lower_filters_in_compound_match_query

* Added check for BetweenClause in _update_tagged_expression

* Added comment

* Addressed PR comments. Simplified location_to_optional_generation.

* Removed check for previous_block is None.

* replaced current_locations with present_locations.

* Made better docstrings

* Removed TODO comments.

* Removed mentions of tag from ir_lowering_match

* Addressed comments.

* Addressed comments.

* Removed redundant check.

* Fixed indentation in within_optional_scope_validation

* Clarifying _prune_traverse_using_omitted_locations

* Clarifying convert_optional_traversals_to_compound_match_query

* Clarifying prune_output_blocks_in_compound_match_query

* Clarifying prune_output_blocks_in_compound_match_query

* Addressed comments for ir_lowering_common.

* Removed hyphens from docstrings.

* Fixed isort errors.

* Addressing comments in convert_optional_traversals_to_compound_match_query

* Fixed string formatting

* Addressing comments predicates to filters.

* Addressing comments predicates to filters.

* fixing docstrings

* Addressing comments predicates to filters.

* Using partial

* Adding blank lines

* Adding comments

* Fixing MATCH indentation

* Added missing tests in `test_compiler` and `test_ir_generation` (#73)

* Added missing tests

* Fixed flake8 error.

* Fixed MATCH formatting

* Addressing comments.

* Fixed overfull lines.

* Fixed line merges.

* Addressed comments from PR.

* Restructuring `MATCH` and `Gremlin` lowering into respective directories (#76)

* Moved lowering steps for optional traversal into new file.

* Restructured lowering code.

* Removed BetweenClause from ir_lowering_match

* removed unused import

* removed unused import

* Renamed files

* Adding missing files

*  Added docstrings, comments and changed variable names. (#77)

* Added docstring, comments and changed variable names.

* Added assertion that optional traverse locations must be mapped to an optional root location.

* Fixing Assertion for Folded OutputContextField in optional traversal.

* Addressing comments.

* Adding missing tests for optional traversals. (#80)

* Added docstring, comments and changed variable names.

* Added assertion that optional traverse locations must be mapped to an optional root location.

* Fixing Assertion for Folded OutputContextField in optional traversal.

* Addressing comments.

* Added more test cases

* Fixed MATCH formatting

* Fixed MATCH formatting

* Fixed MATCH formatting

* Fixed MATCH formatting

* Fixed MATCH formatting

* Fixed FoldScopeLocation Error

* Fixed FoldScopeLocation Error

* Fixing formatting

* Adding comma

* Optional traversal test input data output names fix (#82)

* Fixed output names

* Fixed spouse_name

* Fixed spouse_name

* Fixed lint errors

* Updating documentation (#84)

* Updated documentation

* Updated documentation

* Updated documentation
@amartyashankha amartyashankha deleted the optional_traversal_todos branch May 22, 2018 15:10
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants