Deprecate pgr node network on3.8 #2876
Conversation
WalkthroughThe changes in this pull request focus on the deprecation of the Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant pgr_nodeNetwork
participant MigrationDocs
User->>pgr_nodeNetwork: Call function (v3.8.0+)
pgr_nodeNetwork-->>User: Raise deprecation warning
User->>MigrationDocs: Consult migration guide
MigrationDocs-->>User: Suggest pgr_separateTouching / pgr_separateCrossing
Assessment against linked issues
Possibly related PRs
Suggested reviewers
Poem
✨ Finishing Touches
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. 🪧 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: 5
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (11)
doc/_static/page_history.js(1 hunks)doc/src/migration.rst(1 hunks)doc/src/release_notes.rst(1 hunks)doc/topology/pgr_analyzeGraph.rst(0 hunks)doc/topology/pgr_nodeNetwork.rst(3 hunks)docqueries/topology/nodeNetwork.result(2 hunks)docqueries/utilities/separateCrossing.pg(0 hunks)docqueries/utilities/separateTouching.pg(0 hunks)locale/en/LC_MESSAGES/pgrouting_doc_strings.po(4 hunks)locale/pot/pgrouting_doc_strings.pot(4 hunks)sql/topology/nodeNetwork.sql(2 hunks)
💤 Files with no reviewable changes (3)
- doc/topology/pgr_analyzeGraph.rst
- docqueries/utilities/separateCrossing.pg
- docqueries/utilities/separateTouching.pg
🧰 Additional context used
🧠 Learnings (3)
📓 Common learnings
Learnt from: cvvergara
PR: pgRouting/pgrouting#2744
File: NEWS.md:63-63
Timestamp: 2025-02-06T20:50:00.577Z
Learning: In pgRouting's NEWS.md, the "Removal of SQL deprecated internal functions" section lists SQL signatures being removed, while "Deprecation of internal C/C++ functions" section lists the underlying C/C++ implementations being deprecated. A function can appear in both sections when its SQL signature is removed while its C/C++ implementation is deprecated.
Learnt from: cvvergara
PR: pgRouting/pgrouting#0
File: :0-0
Timestamp: 2025-01-27T01:49:25.212Z
Learning: In pgRouting, when deprecating internal functions:
1. Add deprecation notice in C code using ereport with ERRCODE_FEATURE_NOT_SUPPORTED
2. Update tests with version checks (min_version, min_lib_version)
3. Document changes in NEWS.md under appropriate sections
doc/src/release_notes.rst (3)
Learnt from: cvvergara
PR: pgRouting/pgrouting#2744
File: doc/src/release_notes.rst:94-94
Timestamp: 2025-02-06T20:50:07.967Z
Learning: In pgRouting, changes to functions can happen at two levels:
1. SQL level - removal/addition of function signatures
2. C/C++ level - deprecation/changes to the underlying implementation
These are documented separately in the release notes.
Learnt from: cvvergara
PR: pgRouting/pgrouting#2744
File: NEWS.md:63-63
Timestamp: 2025-02-06T20:50:00.577Z
Learning: In pgRouting's NEWS.md, the "Removal of SQL deprecated internal functions" section lists SQL signatures being removed, while "Deprecation of internal C/C++ functions" section lists the underlying C/C++ implementations being deprecated. A function can appear in both sections when its SQL signature is removed while its C/C++ implementation is deprecated.
Learnt from: cvvergara
PR: pgRouting/pgrouting#0
File: :0-0
Timestamp: 2025-01-27T01:49:25.212Z
Learning: In pgRouting, when deprecating internal functions:
1. Add deprecation notice in C code using ereport with ERRCODE_FEATURE_NOT_SUPPORTED
2. Update tests with version checks (min_version, min_lib_version)
3. Document changes in NEWS.md under appropriate sections
docqueries/topology/nodeNetwork.result (2)
Learnt from: cvvergara
PR: pgRouting/pgrouting#2744
File: NEWS.md:63-63
Timestamp: 2025-02-06T20:50:00.577Z
Learning: In pgRouting's NEWS.md, the "Removal of SQL deprecated internal functions" section lists SQL signatures being removed, while "Deprecation of internal C/C++ functions" section lists the underlying C/C++ implementations being deprecated. A function can appear in both sections when its SQL signature is removed while its C/C++ implementation is deprecated.
Learnt from: cvvergara
PR: pgRouting/pgrouting#0
File: :0-0
Timestamp: 2025-01-27T01:49:25.212Z
Learning: In pgRouting, when deprecating internal functions:
1. Add deprecation notice in C code using ereport with ERRCODE_FEATURE_NOT_SUPPORTED
2. Update tests with version checks (min_version, min_lib_version)
3. Document changes in NEWS.md under appropriate sections
🪛 Biome (1.9.4)
doc/_static/page_history.js
[error] 212-212: expected : but instead found }
Remove }
(parse)
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: Codacy Static Code Analysis
- GitHub Check: Check queries
- GitHub Check: License_check
🔇 Additional comments (12)
doc/src/release_notes.rst (1)
138-139: Properly documents the deprecation of pgr_nodeNetworkThe addition to the "Deprecation of SQL functions" section correctly references issue #2847 and clearly identifies
pgr_nodeNetworkas deprecated in version 3.8. This follows the project's established pattern for documenting deprecations.docqueries/topology/nodeNetwork.result (2)
43-43: Appropriately adds deprecation warningThe added warning message clearly indicates that the
pgr_nodeNetworkfunction is deprecated as of version 3.8.0, which aligns with the PR objectives and helps users understand that they should consider migrating to alternative functions.
145-145: Consistent deprecation warning addedThe same deprecation warning is correctly added here, maintaining consistency in how deprecation messages are displayed to users throughout the codebase.
doc/src/migration.rst (1)
57-71: Well-structured migration guidance providedThe added migration section follows the established pattern of other deprecated functions in the document. It clearly:
- States when the deprecation begins (v3.8.0)
- Explains the previous behavior of the function
- Provides specific guidance on which functions to use as alternatives (
pgr_separateTouchingandpgr_separateCrossing)This information will be valuable for users who need to migrate away from the deprecated function.
doc/topology/pgr_nodeNetwork.rst (2)
41-41: Confirm deprecation note under Availability.The added bullet
* Deprecated function.clearly flags the deprecation. Ensure this aligns with other function docs and downstream tooling picks it up correctly.
50-53: Verify include path for migration snippet.Ensure the path
migration.rstis resolvable by Sphinx. Ifmigration.rstresides indoc/src, you may need a relative path:- .. include:: migration.rst + .. include:: ../src/migration.rstOtherwise, confirm the
includesearch paths inconf.py.locale/en/LC_MESSAGES/pgrouting_doc_strings.po (4)
11-11: Standard timestamp update for translation template.The POT-Creation-Date has been updated to reflect when the translation template was last generated. This is routine maintenance when updating localization files.
4100-4103: Appropriate documentation of deprecation reference.This addition properly documents the deprecation of
pgr_nodeNetworkin version 3.8, including a reference to the relevant GitHub issue (#2847). This follows best practices for documenting deprecations in pgRouting.
4189-4201: Clear migration guidance added.The added migration section effectively:
- Describes the original behavior
- Provides clear migration instructions
- References the replacement functions (
pgr_separateTouchingandpgr_separateCrossing)This guidance helps users transition away from the deprecated function, which is essential when deprecating functionality.
13741-13741: Properly marked as deprecated in function description.The function description now clearly indicates that
pgr_nodeNetworkis deprecated since version 3.8.0, ensuring that users are aware of its status when they consult the documentation.locale/pot/pgrouting_doc_strings.pot (2)
3649-3650: Addition of deprecation notice for pgr_nodeNetwork looks good.This properly documents the issue (#2847) that tracks the deprecation of pgr_nodeNetwork in version 3.8.0, which is important for translation purposes and documentation consistency.
3724-3735: Migration guidance is clear and helpful.The migration section properly explains what the function did before deprecation and directs users to the new recommended functions (
pgr_separateTouchingandpgr_separateCrossing). This aligns with best practices for deprecating functions by providing a clear migration path.
| @@ -209,7 +209,7 @@ var filesArr = [ | |||
| ]), | |||
| new createInfo('pgr_nodeNetwork', '2.0', [ | |||
| { v: '2.1', n: 'src/common/doc/functions/node_network'}, | |||
| { v: '2.3', n: 'src/topology/doc/pgr_nodeNetwork'}, | |||
| { v: '2.3', n: 'src/topology/doc/pgr_nodeNetwork', 3.8}, | |||
There was a problem hiding this comment.
Added version 3.8 to history tracking
The additional 3.8 parameter correctly marks the version when pgr_nodeNetwork was deprecated in the page history tracking system. This will help the documentation system properly represent the status of this function.
However, there appears to be a syntax error in the code. The trailing comma at the end of the line suggests there might be a problem with the structure.
Apply this fix:
- { v: '2.3', n: 'src/topology/doc/pgr_nodeNetwork', 3.8},
+ { v: '2.3', n: 'src/topology/doc/pgr_nodeNetwork', 3.8 },Committable suggestion skipped: line range outside the PR's diff.
🧰 Tools
🪛 Biome (1.9.4)
[error] 212-212: expected : but instead found }
Remove }
(parse)
| single: Topology Family ; pgr_nodeNetwork - Deprecated since 3.8.0 | ||
| single: nodeNetwork - Deprecated since 3.8.0 |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Consider unifying index entries for decluttering.
Currently there are two separate .. index:: directives for pgr_nodeNetwork and nodeNetwork. You may combine them under a single directive for readability:
.. index::
single: Topology Family ; pgr_nodeNetwork - Deprecated since 3.8.0
single: nodeNetwork - Deprecated since 3.8.0|
|
||
| | | ||
|
|
||
| ``pgr_nodeNetwork`` | ||
| ``pgr_nodeNetwork`` - Deprecated since 3.8.0 |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Use Sphinx’s deprecated directive for consistency.
Instead of inline header text, consider adding a .. deprecated:: 3.8.0 directive to leverage Sphinx’s built‐in styling and indexing:
.. deprecated:: 3.8.0
Use :func:`pgr_separateTouching` and :func:`pgr_separateCrossing` instead.| COMMENT ON FUNCTION pgr_createverticestable(TEXT, TEXT, TEXT, TEXT, TEXT) | ||
| IS 'pgr_nodeNetwork deprecated function on v3.8.0'; |
There was a problem hiding this comment.
Correct the COMMENT target and include migration hint
The new COMMENT ON FUNCTION is applied to pgr_createverticestable, but the deprecation notice should target pgr_nodeNetwork. Additionally, the comment should advise users of the replacement API. For example:
COMMENT ON FUNCTION pgr_nodeNetwork(
TEXT, DOUBLE PRECISION, TEXT, TEXT, TEXT, TEXT, BOOLEAN
)
IS 'Deprecated as of v3.8.0; use pgr_separateTouching and/or pgr_separateCrossing instead.';Please update the function name and descriptive text accordingly.
| @@ -63,6 +63,7 @@ DECLARE | |||
| out_table_exists BOOLEAN; | |||
|
|
|||
| BEGIN | |||
| RAISE WARNING 'pgr_nodenetwork(text,double precision,text,text,text,text,boolean) deprecated function on v3.8.0'; | |||
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add guidance to the runtime deprecation warning
The RAISE WARNING at line 66 only flags that pgr_nodeNetwork is deprecated. To improve user experience, enhance the message to:
- Mention the replacement functions (
pgr_separateTouchingandpgr_separateCrossing) - Use a specific SQLSTATE (e.g.,
0A000/FEATURE_NOT_SUPPORTED) so downstream clients can detect deprecation programmatically
Example diff:
- RAISE WARNING 'pgr_nodenetwork(text,double precision,text,text,text,text,boolean) deprecated function on v3.8.0';
+ RAISE WARNING
+ 'Function pgr_nodeNetwork is deprecated as of version 3.8.0; use pgr_separateTouching and/or pgr_separateCrossing instead.'
+ USING ERRCODE = '0A000';
Fixes #2847 .
Changes proposed in this pull request:
pg_nodeNetowrk@pgRouting/admins
Summary by CodeRabbit
Documentation
pgr_nodeNetworkfunction as deprecated since version 3.8.0, including deprecation notices, migration guidance, and removal of references in related docs and translation files.pgr_separateTouchingand/orpgr_separateCrossingas alternatives.Bug Fixes
pgr_nodeNetwork.Chores