Feat/136 simple graph segmentation for damages

Deltares / ra2ce

RA2CE helps to quantify resilience of critical infrastructure networks, prioritize interventions and adaptation measures and select the most appropriate action perspective to increase resilience considering future conditions.

https://deltares.github.io/ra2ce/

Other

9 stars 2 forks source link

Feat/136 simple graph segmentation for damages #539

Closed sahand-asgarpour closed 1 month ago

sahand-asgarpour commented 1 month ago

Issue addressed

Solves #136

Code of conduct

[x] I HAVE NOT added sensitive or compromised (test) data to the repository.
[x] I HAVE NOT added vulnerabilities to the repository.
[x] I HAVE discussed my solution with (other) members of the RA2CE team.

What has been done?

Explain how you addressed the resolution of the related issue, what choices you made and why.

Checklist

[x] Code is formatted using our custom black and isort definitions.
[x] Tests are either added or updated.
[x] Branch is up to date with master.
[ ] Updated documentation if needed.

Additional Notes (optional)

Add any additional notes or information that may be helpful.

ArdtK commented 3 weeks ago

@sahand-asgarpour in NetworkWrapperProtocol you added method segment_graph. It is not correct to add implemented methods to a protocol. A protocol is meant to only contain signatures (definitions). If you would like to have such a method that is used in all implementing classes, you should create a base class next to the protocol.

sahand-asgarpour commented 3 weeks ago

@sahand-asgarpour in NetworkWrapperProtocol you added method segment_graph. It is not correct to add implemented methods to a protocol. A protocol is meant to only contain signatures (definitions). If you would like to have such a method that is used in all implementing classes, you should create a base class next to the protocol.

I understand. Thank you for the clarification. I make an issue for that to be picked up later. Is it an idea?

Just made an issue.

Carsopre commented 1 week ago

This PR should have not been merged when there's a configuration build failing!