Update from task 7164cb1f-2349-465a-8217-35a326681d96#4
Conversation
- Update src/elevation/selector.rs to handle global providers correctly and fix test assertion for Copernicus selection - Modify src/elevation/selector.rs to properly include CopernicusDem30 in provider selection when no regional matches exist - Fix src/overture.rs by adding missing scale parameter to process_partition_file_by_collection function - Correct src/overture.rs to replace nonexistent CoordTransformer::identity() and LLBBox::new_unchecked() APIs with valid constructor calls - Update .gitignore to simplify ignored file patterns
ⓘ You've reached your Qodo monthly free-tier limit. Reviews pause until next month — upgrade your plan to continue now, or link your paid account if you already have one. |
![amazon-q-developer[bot]](https://avatars.githubusercontent.com/in/1220912?s=60&v=4){kind=small}
There was a problem hiding this comment.
Summary
This PR replaces unsafe LLBBox::new_unchecked() calls with safe LLBBox::new() validation in four locations, properly handling errors with .ok()? pattern. The safety improvements in src/overture.rs correctly validate coordinate bounds before bbox construction.
The .gitignore simplification removes many useful patterns that prevent accidental commits of build artifacts, IDE files, and system files. While this doesn't block compilation, it increases the risk of repository pollution.
You can now have the agent implement changes and create commits directly on your pull request's source branch. Simply comment with /q followed by your request in natural language to ask the agent to make changes.
| # Compiled and build artifacts | ||
| *.pyc |
There was a problem hiding this comment.
🛑 Configuration Risk: Removing essential .gitignore entries allows build artifacts, IDE files, system files, and logs to be accidentally committed to the repository.
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request updates the .gitignore file and replaces unchecked bounding box creation with validated creation in src/overture.rs. The .gitignore changes incorrectly include markdown delimiters and remove many necessary ignore patterns. In src/overture.rs, the new LLBBox validation logic is redundantly executed inside closures for every feature (buildings, roads, water, and landuse), which should be moved outside the closures to improve performance.
| ``` | ||
| # Compiled and build artifacts | ||
| *.pyc | ||
| __pycache__/ | ||
| *.o | ||
| *.obj | ||
| *.exe | ||
| *.dll | ||
| *.so | ||
| *.a | ||
| *.out | ||
| dist/ | ||
| build/ | ||
| target/ | ||
|
|
||
| # Dependencies | ||
| .venv/ | ||
| venv/ | ||
| node_modules/ | ||
| .mypy_cache/ | ||
| .pytest_cache/ | ||
| .gradle/ | ||
|
|
||
| # Logs and temp files | ||
| *.log | ||
| *.tmp | ||
| *.swp | ||
| *.swo | ||
|
|
||
| # Environment | ||
| .env | ||
| .env.local | ||
| .env.* | ||
|
|
||
| # Editors | ||
| .vscode/ | ||
| .idea/ | ||
|
|
||
| # System files | ||
| .DS_Store | ||
| Thumbs.db | ||
|
|
||
| # Coverage | ||
| coverage/ | ||
| htmlcov/ | ||
| .coverage | ||
|
|
||
| # Compressed files | ||
| *.zip | ||
| *.gz | ||
| *.tar | ||
| *.tgz | ||
| *.bz2 | ||
| *.xz | ||
| *.7z | ||
| *.rar | ||
| *.zst | ||
| *.lz4 | ||
| *.lzh | ||
| *.cab | ||
| *.arj | ||
| *.rpm | ||
| *.deb | ||
| *.Z | ||
| *.lz | ||
| *.lzo | ||
| *.tar.gz | ||
| *.tar.bz2 | ||
| *.tar.xz | ||
| *.tar.zst | ||
| **/*.rs.bk | ||
| ``` No newline at end of file |
There was a problem hiding this comment.
The .gitignore file contains markdown code block delimiters (```), which is incorrect for a configuration file. Furthermore, this change deletes a large number of essential ignore patterns for build artifacts, dependencies, and IDE settings. These should be restored to maintain project hygiene.
target/
**/*.rs.bk
| return None; | ||
| } | ||
| building_to_processed_way(&b, coord_transformer, &LLBBox::new_unchecked(target_min_lat, target_min_lng, target_max_lat, target_max_lng)) | ||
| let bbox = LLBBox::new(target_min_lat, target_min_lng, target_max_lat, target_max_lng).ok()?; |
There was a problem hiding this comment.
The LLBBox is being reconstructed and validated for every single building in the collection (up to 100,000 iterations). Since the bounding box coordinates are constant for the entire row group, this instance should be created once outside the filter_map closure to improve performance.
| return None; | ||
| } | ||
| road_to_processed_way(&r, coord_transformer, &LLBBox::new_unchecked(target_min_lat, target_min_lng, target_max_lat, target_max_lng)) | ||
| let bbox = LLBBox::new(target_min_lat, target_min_lng, target_max_lat, target_max_lng).ok()?; |
There was a problem hiding this comment.
The LLBBox is being reconstructed for every road feature. This should be moved outside the filter_map closure to avoid redundant calculations.
| return None; | ||
| } | ||
| water_to_processed_way(&w, coord_transformer, &LLBBox::new_unchecked(target_min_lat, target_min_lng, target_max_lat, target_max_lng)) | ||
| let bbox = LLBBox::new(target_min_lat, target_min_lng, target_max_lat, target_max_lng).ok()?; |
There was a problem hiding this comment.
The LLBBox is being reconstructed for every water feature. This should be moved outside the filter_map closure to avoid redundant calculations.
| return None; | ||
| } | ||
| landuse_to_processed_way(&l, coord_transformer, &LLBBox::new_unchecked(target_min_lat, target_min_lng, target_max_lat, target_max_lng)) | ||
| let bbox = LLBBox::new(target_min_lat, target_min_lng, target_max_lat, target_max_lng).ok()?; |
There was a problem hiding this comment.
The LLBBox is being reconstructed for every land use feature. This should be moved outside the filter_map closure to avoid redundant calculations.
…lbbox-validation Support multiple Copernicus TIFF sample formats, reuse bbox in Overture parsing, and expand .gitignore
2 participants
This PR was created by qwen-chat coder for task 7164cb1f-2349-465a-8217-35a326681d96.