Rework renaming columns in | timechart #94
Conversation
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
eemhu
reviewed
Feb 18, 2025
eemhu
approved these changes
Feb 18, 2025
Contributor
Author
|
Merge with caution. Using this in PTH-10 will break timechart, so refactoring has to be done there after merging. |
Member
|
@eemhu please coordinate this work or close pr if it is no longer necessary. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
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.
Fixes #92 .
Moved renaming columns in aggregations and evaluations under new grammar rules together with the column that is going to be renamed. This makes it possible to traverse the parse tree in any order in PTH-10 without losing information about which renaming instruction corresponds to which column. Previously the rename instruction had to be parsed directly after the corresponding column, which makes parsing unnecessarily difficult.
Created tests to assert the parse tree is correct in these cases.
The grammar should not have changed at all in this PR! Only the grammar rules change, which creates a need for changes in PTH-10 when traversing the parse tree. DPL language stays the same though.
Evaluation functions had no tests yet in timechart. New issue #93 was opened about those, as a bug was found when testing them. The test introduced in this PR makes sure that the renaming of eval functions works as intended.