Repository navigation
geojson: holes, features with no place, both types of id, and bbox - #172
Merged
Merged
Conversation
Four settings, all off by default, so files made with the earlier settings keep their bytes (the six golden cases are unchanged). - holes: up to 100 000 holes of four points in every polygon, each running the other way round to its outline. An outline with holes keeps a band round its middle free by construction, and every hole keeps a share of its cell clear - a one-step margin is below what a double tells apart at fifteen places. An outline widened for holes spans at most half the globe, so no edge reads as the way round the other side. Needs 6 or more vertices. - unlocated: every fifth feature (some) or every one (all) has a geometry of null. - ids: number, string (f2), mixed or none. GDAL keeps only the numbers of a mixed file and warns about shared ids - the guard allows that warning and nothing else. - bbox: on the collection and every feature with a place, the collection's after its features because the generator streams. A box across the antimeridian has its west edge greater than its east edge, and one round the globe is -180 to 180. GDAL ignores bbox, so the structural checker rebuilds every box from the positions. The shapely check skips null geometries and unwraps a whole shape from one anchor, so a hole past 180 stays inside its outline. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configuration
You can disable this status message by setting the Use the checkbox below for a quick retry:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configuration
⛔ Files ignored due to path filters (22)
📒 Files selected for processing (40)
Included review availability: This review used your included allowance. Your plan provides up to 1 included review per hour; 0 remain after this review. 📜 Recent review details⏰ Context from checks skipped due to timeout. (16)
🧰 Additional context used📚 Code guidelines (1)📓 Path-based instructions (15)Applies to text shown to the user (labels, buttons, tooltips, placeholders, dialogs, errors, status messages, empty states, translations).⚙️ CodeRabbit configuration file Files:
Verify tests check real behavior and would fail if the implementation were broken.⚙️ CodeRabbit configuration file Files:
These are end-user desktop applications.⚙️ CodeRabbit configuration file Files:
Performance is a known weak spot of these projects.⚙️ CodeRabbit configuration file Files:
Applies only to code that builds or styles a GUI.⚙️ CodeRabbit configuration file Files:
User-facing changelog.⚙️ CodeRabbit configuration file Files:
Domain: test file generator (Go; `tfg` CLI and `tfg-gui` Fyne window over one engine).⚙️ CodeRabbit configuration file Files:
SECURITY, HIGH PRIORITY.⚙️ CodeRabbit configuration file Files:
These apps are QA/developer tools.⚙️ CodeRabbit configuration file Files:
Source of the public project website (generated output is excluded from review).⚙️ CodeRabbit configuration file Files:
Go code.⚙️ CodeRabbit configuration file Files:
Check that documentation matches the actual code in this PR: commands, flags, config keys, file paths, build steps and examples must exist.⚙️ CodeRabbit configuration file Files:
Python code.⚙️ CodeRabbit configuration file Files:
All code in this repository is written by an AI coding agent (Claude Code).⚙️ CodeRabbit configuration file Files:
Source excerpt: **Words a user reads are English, with a flat hyphen and no semicolons.**📄 CodeRabbit inference engine (CONTRIBUTING.md) Files:
🪛 LanguageToolCHANGELOG.md[style] ~93-~93: In formal contexts, the form “around” is more common (ROUND_AROUND) 📝 WalkthroughWalkthroughGeoJSON generation now supports polygon holes, unlocated features, configurable feature IDs, and feature and collection bounding boxes. Settings, geometry serialization, validation, reference-tool checks, interface text, and documentation cover these options. ChangesGeoJSON output
Priority: ➖ Normal Estimated code review effort: 4 (Complex) | ~60 minutes Change: Feature Sequence Diagram(s)sequenceDiagram
participant GeoJSONSettings
participant records
participant drawing
participant emitter
GeoJSONSettings->>records: provide geometry, ID, and bbox settings
records->>drawing: draw feature and accumulate extent
records->>emitter: write feature ID and optional feature bbox
records->>emitter: write collection bbox when an extent is set
Suggested labels: Merge Risk: ⚪ Minimal · up to No actionable issue identified in the new GeoJSON options; mergeable after normal checks. 🚥 Pre-merge checks | ✅ 13 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (13 passed)
Full details: Safe File ParsingExplanation
Resolution Replace
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
…ecker's box only when asked The smallest closing feature built the end of the collection into a new buffer for every kind and count it measured - fourteen allocations a file with mixed, past the ceiling of the guard that keeps a file out of memory. It now reuses the buffer the closing feature already has. The structural checker worked out every feature's box even with bbox off and kept a copy of every position to do it. It now keeps a running minimum and maximum, only when a bbox was ordered, and walks edges a pair at a time. Measured, the copy did not set the checker's peak - the parsed document and the rebuilt layout do - so the comment says so. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
Four new settings for the
geojsonformat, all off by default - files made with the earlier settings keep their bytes (the six existing golden cases are unchanged).What a user gets
holes(0 to 100 000): every polygon gets that many holes of four points, each running the other way round to its outline as RFC 7946 asks. Needs 6 or morevertices. Fewer fit at precision 3 or less, and asking for more is refused with the number that fits.unlocated(none,some,all): every fifth feature, or every one, has a geometry ofnull.ids(number,string,mixed,none): the id as a number (as before), as a string such asf2, both in turn, or left out. Measured: GDAL keeps only the numbers of a mixed file and warns that several features share an id.bbox: the collection and every feature with a place carry the box their coordinates lie in. The collection's box comes after its features (its extent is known only after the last one). A box across the antimeridian has its west edge greater than its east edge (RFC 7946 §5.2), and one whose shapes reach all the way round is -180 to 180 (§5.3).How it is checked
bboxentirely (a wrong box and one of three numbers both opened without a word), and neither GDAL nor shapely looks at which way a hole runs. The structural checker (check_geojson) is the witness for both: it rebuilds every box from the positions and checks every hole's winding. It also checks that no edge spans half the globe, and it reports the shape of the collection's box so a guard can assert that a crossing box and one round the globe were reached.nullgeometries and unwraps a whole shape from one anchor, so a hole past 180 stays inside its outline.Not done here
🤖 Generated with Claude Code
Summary by CodeRabbit