Skip to content
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

Add reorder topologies command #3296

Merged
merged 8 commits into from
Nov 3, 2022
Merged

Add reorder topologies command #3296

merged 8 commits into from
Nov 3, 2022

Conversation

LePetitTim
Copy link
Contributor

No description provided.

@LePetitTim LePetitTim force-pushed the add_command_reorder branch 2 times, most recently from de80d9b to aa27b77 Compare October 14, 2022 15:13
@codecov
Copy link

codecov bot commented Oct 14, 2022

Codecov Report

Base: 98.13% // Head: 98.14% // Increases project coverage by +0.00% 🎉

Coverage data is based on head (18c4f91) compared to base (e8e2fd1).
Patch coverage: 100.00% of modified lines in pull request are covered.

Additional details and impacted files
@@           Coverage Diff           @@
##           master    #3296   +/-   ##
=======================================
  Coverage   98.13%   98.14%           
=======================================
  Files         273      274    +1     
  Lines       19890    19968   +78     
=======================================
+ Hits        19520    19598   +78     
  Misses        370      370           
Impacted Files Coverage Δ
...rek/core/management/commands/reorder_topologies.py 100.00% <100.00%> (ø)

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

@cypress
Copy link

cypress bot commented Oct 14, 2022



Test summary

24 0 0 0


Run details

Project Geotrek-admin
Status Passed
Commit 70d172e ℹ️
Started Nov 2, 2022 6:29 PM
Ended Nov 2, 2022 6:34 PM
Duration 04:27 💡
OS Linux Ubuntu - 20.04
Browser Electron 94

View run in Cypress Dashboard ➡️


This comment has been generated by cypress-bot as a result of this project's GitHub integration settings. You can manage this integration in this project's settings in the Cypress Dashboard

@camillemonchicourt
Copy link
Member

Related to #2515

1 ⠞ ⠳ 2
⠞ ⠳
"""
self.path_1 = PathFactory.create(geom=LineString(Point(700000, 6600000), Point(700100, 6600100),
Copy link
Contributor

@marcantoinedupre marcantoinedupre Oct 25, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Only for your information, since python3.6, you can add '_' in numeric litterals to make them more readable (the '_' characters are ignored when parsing the numeric litterals).
For instance : LineString(Point(700_000, 6_600_000), Point(700_100, 6_600_100))

"""
Part A

⠳ 🡥
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No big deal because it has something to do with my setup : the up arrow character is not displayed neither on Github or PyCharm for me. What did you use?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.


def get_geometries(self):
geometries = []
for pathagg in PathAggregation.objects.all():
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For consistency I would filter the queryset with the topology's ID in the same way you do in the tests.

call_command('reorder_topologies', stdout=output)
self.assertEqual(f'Topologies with errors :\nTREK id: {topo.pk}\n', output.getvalue())
geometries = self.get_geometries()
self.assertEqual(geometries, [LineString((700045, 6600045), (700050, 6600050), srid=2154),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You have added assertions that test bad values ;) IMHO I would stop the test after asserting the error.

@marcantoinedupre marcantoinedupre self-requested a review October 25, 2022 15:27
Copy link
Contributor

@marcantoinedupre marcantoinedupre left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good job 👍

@@ -5,10 +5,14 @@ CHANGELOG
2.89.1+dev (XXXX-XX-XX)
-----------------------

**New features**

- Add new command to reorder pathaggregations of topologies
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you specify the name of the command, as I am not sure to find its name...
Thanks.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am adding documentation then is it necessary ?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK thanks

@LePetitTim LePetitTim requested review from marcantoinedupre and a team November 2, 2022 12:39
[--version] [-v {0,1,2,3}] [--settings SETTINGS] [--pythonpath PYTHONPATH] [--traceback] [--no-color] [--force-color] [--skip-checks]
point_layer

Load a layer with point geometries in te structure model
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
Load a layer with point geometries in te structure model
Load a layer with point geometries in the structure model

[-v {0,1,2,3}] [--settings SETTINGS] [--pythonpath PYTHONPATH] [--traceback] [--no-color] [--force-color] [--skip-checks]
point_layer

Load a layer with point geometries in te structure model
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
Load a layer with point geometries in te structure model
Load a layer with point geometries in the signage model

Remove duplicate paths
----------------------

It can happens during additions of paths with commands or directly in the application to have duplicate paths.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
It can happens during additions of paths with commands or directly in the application to have duplicate paths.
Duplicate paths can appear while adding paths with commands or directly in the application.

----------------------

It can happens during additions of paths with commands or directly in the application to have duplicate paths.
Duplicate paths can cause some problems of routing for topologies, it can generates topologies representing multilinestring.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
Duplicate paths can cause some problems of routing for topologies, it can generates topologies representing multilinestring.
Duplicate paths can cause some problems of routing for topologies, it can generate corrupted topologies (that become MultiLineStrings instead of LineStrings).


You have to run ``sudo geotrek remove_duplicate_paths``

During the process of the command every topologies on a duplicate path will be set on the original path and the duplicate path deleted.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
During the process of the command every topologies on a duplicate path will be set on the original path and the duplicate path deleted.
During the process of the command, every topology on a duplicate path will be set on the original path, and the duplicate path will be deleted.

Reorder topologies
------------------

Every topologies have informations of which path they pass on and in which order.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
Every topologies have informations of which path they pass on and in which order.
All topologies have information about which path they go through on and in which order.

------------------

Every topologies have informations of which path they pass on and in which order.
Actually, when an path is split in 2 by an other path, database add a new path.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
Actually, when an path is split in 2 by an other path, database add a new path.
Actually, when a path is split in 2 by another path, a new path is added to the database.


Every topologies have informations of which path they pass on and in which order.
Actually, when an path is split in 2 by an other path, database add a new path.
We need to add informations for every topologies which need to pass on this new path.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
We need to add informations for every topologies which need to pass on this new path.
We need to add information for all topologies that need to go through this new path.

This is badly managed at the moment, especially for the order of passage of the paths.
``sudo geotrek reorder_topologies``

It removes a lot of useless information which can accelerate the process of edition of topologies afterward.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
It removes a lot of useless information which can accelerate the process of edition of topologies afterward.
It removes a lot of useless information which can accelerate the process of editing topologies afterward.

It removes a lot of useless information which can accelerate the process of edition of topologies afterward.


During the process of this command, it tries to find a good order of passage on the paths which create
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
During the process of this command, it tries to find a good order of passage on the paths which create
During the process of this command, it tries to find a good order of passage on the paths which creates



During the process of this command, it tries to find a good order of passage on the paths which create
only one Linestring from the start to the end. It uses the wrong orders as much as possible. This command use the same algorithm to generate one Linestring
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
only one Linestring from the start to the end. It uses the wrong orders as much as possible. This command use the same algorithm to generate one Linestring
only one Linestring from start to end. It stays as close as possible to the corrupted order. This command uses the same algorithm to generate one Linestring


.. danger::
It can happens that this algorithm can't find any solution and will genereate a MultiLineString.
This will be display at the end of the reorder
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
This will be display at the end of the reorder
This will be displayed at the end of the reorder

-----------------------


You can set up automatic commands by creating a file ``/etc/cron.d/geotrek_command`` that contains:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
You can set up automatic commands by creating a file ``/etc/cron.d/geotrek_command`` that contains:
You can set up automatic commands by creating a `cron` file under ``/etc/cron.d/geotrek_command`` that contains:

--skip-checks Skip system checks.

.. danger::
You can't chose for each choice which set of category you want to unset structures. Every categories of each choice will
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
You can't chose for each choice which set of category you want to unset structures. Every categories of each choice will
You can't chose for each choice which set of category you want to unset structures, it will happen for all categories



Firstly, it creates (or finds) the category without structure associated.
Secondly, every element with this category get this new category.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
Secondly, every element with this category get this new category.
Secondly, every element with this old category gets assigned to this new category.

You can't chose for each choice which set of category you want to unset structures. Every categories of each choice will


Firstly, it creates (or finds) the category without structure associated.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
Firstly, it creates (or finds) the category without structure associated.
Firstly, if a categroy is linked to a structure, it creates the same category but with no structure associated.

Unset structure on categories
-----------------------------

If you need to remove some similar categories with different structures you may need to remove the structure of all categories
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pas très clair

Suggested change
If you need to remove some similar categories with different structures you may need to remove the structure of all categories
Use this command if you wish to undo linking categories to structures for some models

@LePetitTim LePetitTim requested a review from Chatewgne November 3, 2022 10:56
@LePetitTim LePetitTim merged commit 33c342b into master Nov 3, 2022
@LePetitTim LePetitTim deleted the add_command_reorder branch November 3, 2022 11:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants