Topology v2 #136
Topology v2 #136
Conversation
|
Caution Review failedThe pull request is closed. 📝 Walkthrough📝 WalkthroughWalkthroughThe pull request involves the deletion of the file Changes
Possibly related PRs
Poem
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
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Outside diff range and nitpick comments (4)
postgis-intro/sources/en/topology_base_types.rst (4)
6-7: Fix grammar in prerequisite statementThe sentence has incorrect grammar. Consider revising to:
"Before reading this document, please check at least one of these documents:"
25-27: Enhance type definitions for clarityConsider making the definitions more precise:
- Node: "A 0-dimensional point that serves as a start or end point for edges"
- Edge: "A directed 1-dimensional linestring that connects exactly two nodes"
- Face: "A 2-dimensional polygon bounded by a closed sequence of edges"
38-41: Clarify Universal Face explanationThe explanation could be more precise. Consider:
"The Universal Face is a special face that represents the infinite area surrounding all other topological elements. It's not just empty space - it's a well-defined face that contains all other topology elements and has ID 0."
74-74: Fix typo in edge description"organge" should be "orange"
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
⛔ Files ignored due to path filters (3)
postgis-intro/sources/en/topology/boros_universal_face.pngis excluded by!**/*.pngpostgis-intro/sources/en/topology/face_next_direction.pngis excluded by!**/*.pngpostgis-intro/sources/en/topology/next_edge.pngis excluded by!**/*.png
📒 Files selected for processing (2)
postgis-intro/sources/en/topology_base.rst(0 hunks)postgis-intro/sources/en/topology_base_types.rst(1 hunks)
💤 Files with no reviewable changes (1)
- postgis-intro/sources/en/topology_base.rst
🔇 Additional comments (1)
postgis-intro/sources/en/topology_base_types.rst (1)
1-186: Overall documentation structure is well-organized
The document provides a comprehensive introduction to topology concepts with good use of visual aids and examples. The explanations are generally clear and the progression from basic concepts to more complex ones is logical.
However, there are several areas that need attention:
- Multiple placeholder section headers need to be replaced
- Some technical definitions could be more precise
- A few typos and incorrect column names need to be fixed
Despite these issues, the document is a significant improvement over the previous version and provides valuable information for understanding PostGIS topology.
|
|
||
| To correctly represent a topology and their forms, there are some definitions that are used to construct the tables that store everything, the edge is on the more complex ones. | ||
|
|
||
| All the information about edges are stored on the edge_data table, in your custom topology schema, and which information do we need in the edges? basically its nodes and faces information, edge is the primitive who connects both. |
There was a problem hiding this comment.
I'm not sure I'd document edge_data in the workshop, as it is mostly an implementation detail. The important thing to document is the edge view, which is also defined by the standard.
There was a problem hiding this comment.
mm, this docs are mainly focused on the implementations, how to interpret, to be able how to use.
Topology is harder than SF, and does not has a lot of operation, any operation we want to create even for personal use we can need this information.
even on the start I wrote:
In this workshop, we will check some fundamentals of topology, its basics and definitions. It is not intended to be a way to use them directly, is to understand it to then be able to use it easily.
I'll write clearly the edge view.
There was a problem hiding this comment.
In the Topology Hierarchy I changed how I wrote it, has first a section closer to the User, and then more specific aspects of the concepts.
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Outside diff range and nitpick comments (8)
postgis-intro/sources/en/topology_base_types.rst (3)
6-7: Fix grammar in the introductionThe sentence should be rephrased for better clarity.
-Before read this document, at least check one of this documents: +Before reading this document, please review at least one of these documents:
31-31: Fix grammar and improve clarityThe sentence structure needs improvement.
-Is good to know, until now Postgis will save all this information internally, each type has its own unique ID, and with it we can edit and change them using the available functions, for example with ST_RemEdgeNewFace you can remove an edge and create a new face. +It's important to note that PostGIS stores all this information internally. Each type has its own unique ID, which can be used to edit and modify them using available functions. For example, ST_RemEdgeNewFace can be used to remove an edge and create a new face.
33-33: Consider expanding on advanced conceptsThe sentence introduces important concepts (Layers, TopoGeometry, TopoElement) but doesn't explain them. Consider adding brief descriptions or linking to the relevant sections where these concepts are explained in detail.
postgis-intro/sources/en/topology_topo_types.rst (5)
1-5: Consider adding a table of contentsThe document would benefit from a table of contents to improve navigation. In RST, you can add this using the
.. contents::directive... _topology: Topology and Geometry Representation ==================================== + +.. contents:: Table of Contents + :depth: 2
53-66: Enhance features section with a table formatConsider using an RST table to present the feature types and their identifiers more clearly.
Features -------- +.. list-table:: Feature Types + :header-rows: 1 + + * - Type ID + - Feature + * - 1 + - Nodes + * - 2 + - Edges + * - 3 + - Faces + * - 4 + - Geometry Collections - -Features will represent which type the set of geometries will be, is not important if you use TopoGeometries or Primitives, in the end a Layer can contain directly or indirectly any of this sets: - -- (1) Nodes -- (2) Edges -- (3) Faces -- (4) Geometry Collections
67-91: Improve organization of hierarchical layers sectionThe section could benefit from a more structured approach to explaining parent-child relationships.
Consider:
- Adding a clear definition of hierarchical layers upfront
- Using bullet points for key concepts
- Adding a "Limitations" subsection for lines 88-90
Example structure:
Hierarchical Layers, Children and Parents --------------------------------------- Definition ~~~~~~~~~~ A hierarchical layer system in PostGIS topology allows for the organization of geometric elements in a parent-child relationship. Key Concepts ~~~~~~~~~~~ * Parent layers contain larger groupings of geometric elements * Child layers contain the smaller components that make up parent elements * Each layer can only have one child layer within the same topology schema Limitations ~~~~~~~~~~ * Cannot reference partial TopoGeometries * Cannot mix layers from different hierarchies * Current implementation restricts multiple child layers
115-176: Standardize header formatting and improve structureThe section uses inconsistent header decorations and could benefit from better organization.
- Standardize header decorations:
Relation's Table ----------------- +=============== Keys and Identifiers we know now -<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<< +-------------------------------- Implicit identifiers on Keys ->>>>>>>>>>>>>>>>>>>>>>>>>>>> +---------------------------
- Consider adding a summary table for the relation table's structure:
.. list-table:: Relation Table Structure :header-rows: 1 * - Column - Description * - topology_id - Identifier of the topology (implicit) * - layer_id - Identifier of the layer * - element_id - Identifier of the element * - element_type - Type of the element
177-229: Standardize formatting and enhance technical explanationsThe TopoGeometry components section needs consistent formatting and clearer technical explanations.
- Standardize header decorations:
Find Components of a TopoGeometry -<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<< +-------------------------------- Reading TopoElements -<<<<<<<<<<<<<<<<<<<< +-------------------
- Consider adding a flowchart or diagram showing the process of reading TopoElements:
.. code-block:: text TopoGeometry ↓ Check child_id ↓ ┌─────┴─────┐ ↓ ↓ NULL Not NULL ↓ ↓ Primitive TopoGeometry ID Key
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
⛔ Files ignored due to path filters (7)
postgis-intro/sources/en/topology/components.pngis excluded by!**/*.pngpostgis-intro/sources/en/topology/geomtable2topotable.pngis excluded by!**/*.pngpostgis-intro/sources/en/topology/hierarchy.pngis excluded by!**/*.pngpostgis-intro/sources/en/topology/read_topoelement.pngis excluded by!**/*.pngpostgis-intro/sources/en/topology/topo_keys.pngis excluded by!**/*.pngpostgis-intro/sources/en/topology/topogeo2components.pngis excluded by!**/*.pngpostgis-intro/sources/en/topology/topology_main_concept.pngis excluded by!**/*.png
📒 Files selected for processing (2)
postgis-intro/sources/en/topology_base_types.rst(1 hunks)postgis-intro/sources/en/topology_topo_types.rst(1 hunks)
🔇 Additional comments (1)
postgis-intro/sources/en/topology_base_types.rst (1)
183-183:
Fix incorrect column name
The column name abs_next_right_face should be abs_next_right_edge to match the actual column name in the edge_data table.
-abs_next_right_face: Next edge who build the face on the right.
+abs_next_right_edge: Next edge who builds the face on the right.Likely invalid or redundant comment.
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
|
@strk The documentation is ready! |
Improve explanation of basic types in topology.
And include geometry representation.