-
Notifications
You must be signed in to change notification settings - Fork 2
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
chore(engine): update folder and file path reader names and add new data conversion workflow #631
Conversation
…ata conversion workflow
WalkthroughThe changes involve modifications to several YAML configuration files related to data workflows and node definitions. Key updates include the renaming of nodes for improved clarity and consistency, along with the introduction of a new workflow YAML file. Existing workflows have been adjusted to reflect new parameters and structural changes, particularly in the Changes
Possibly related PRs
Suggested labels
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
Documentation and Community
|
✅ Deploy Preview for reearth-flow ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
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.
Actionable comments posted: 1
🧹 Outside diff range and nitpick comments (3)
engine/runtime/examples/plateau/testdata/graphs/folder_and_file_path_reader.yml (1)
Line range hint
1-83
: Well-structured naming improvementsThe renaming changes follow a consistent pattern that:
- Removes arbitrary numbered suffixes (01, 02)
- Adds descriptive suffixes based on node functionality
- Maintains the workflow structure while improving readability
This makes the configuration more maintainable and self-documenting.
engine/runtime/examples/plateau/testdata/workflow/data-convert/05-fld/workflow.yml (2)
1-18
: Add parameter documentation and validationThe workflow configuration would benefit from:
- Documentation comments explaining each parameter's purpose and expected values
- Default values or validation rules for critical parameters like
cityGmlPath
andoutputPath
Add documentation comments like this:
with: + # Path to the CityGML input file cityGmlPath: + # City code identifier cityCode: + # Path to codelists directory codelistsPath:
168-218
: Add workflow diagram documentationThe edge definitions create a complex processing graph. Consider:
- Adding a visual workflow diagram (e.g., using Mermaid or PlantUML)
- Documenting the purpose of each processing stage
Example documentation using Mermaid:
graph TD A[AttributeReader] --> B[AttributeMapperFilePath] B --> C[FileWriterTsv] A --> D[FeatureTransformer] D --> E[FeatureReader] E --> F[LodSplitter] F -->|lod1| G[AttributeMapperLod0-2] G --> H[MvtWriter] F -->|lod3| I[AttributeMapperLod3] I --> J[VerticalReprojector] J --> K[3DTilesWriter]
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (3)
engine/runtime/examples/plateau/testdata/graphs/folder_and_file_path_reader.yml
(4 hunks)engine/runtime/examples/plateau/testdata/workflow/data-convert/04-luse-lsld/workflow.yml
(0 hunks)engine/runtime/examples/plateau/testdata/workflow/data-convert/05-fld/workflow.yml
(1 hunks)
💤 Files with no reviewable changes (1)
- engine/runtime/examples/plateau/testdata/workflow/data-convert/04-luse-lsld/workflow.yml
🔇 Additional comments (6)
engine/runtime/examples/plateau/testdata/graphs/folder_and_file_path_reader.yml (5)
Line range hint 5-11
: LGTM: Improved node naming
The removal of the "01" suffix from FilePathExtractor
makes the name clearer while maintaining the node's functionality.
Line range hint 14-21
: LGTM: Clear and specific filter node
The simplified name FeatureFilter
better represents its purpose of filtering GML files.
24-29
: LGTM: Consistent naming for PLATEAU extractor
The removal of the "-01" suffix maintains naming consistency while preserving the domain-specific prefix "PLATEAU".
Line range hint 32-39
: LGTM: More descriptive filter name
The rename from FeatureFilter02
to FeatureFilterByPackage
better describes its specific purpose of filtering by package name. The null coalescing operator provides good protection against undefined targetPackages.
Line range hint 42-48
: LGTM: Descriptive counter node name
The rename from FeatureCounter01
to FeatureCounterByUdxDirs
clearly indicates its purpose of counting features grouped by UDX directories.
engine/runtime/examples/plateau/testdata/workflow/data-convert/05-fld/workflow.yml (1)
121-167
: Clarify zoom level strategy for MVT and 3DTiles
The MVT writer (zoom 8-16) and 3DTiles writer (zoom 15-18) have overlapping zoom levels (15-16). This might lead to:
- Duplicate data generation
- Potential inconsistencies in visualization
Please verify if this overlap is intentional and document the rationale.
Overview
What I've done
What I haven't done
How I tested
Screenshot
Which point I want you to review particularly
Memo
Summary by CodeRabbit
New Features
Improvements
AttributeMapperLuse
node for better usability.Bug Fixes