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

Feat/136 simple graph segmentation for damages #539

Merged

Conversation

sahand-asgarpour
Copy link
Contributor

@sahand-asgarpour sahand-asgarpour commented Jul 29, 2024

Issue addressed

Solves #136

Code of conduct

  • I HAVE NOT added sensitive or compromised (test) data to the repository.
  • I HAVE NOT added vulnerabilities to the repository.
  • 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

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

Additional Notes (optional)

Add any additional notes or information that may be helpful.

@sahand-asgarpour sahand-asgarpour linked an issue Jul 29, 2024 that may be closed by this pull request
2 tasks
@sahand-asgarpour sahand-asgarpour marked this pull request as ready for review July 29, 2024 13:53
@sahand-asgarpour sahand-asgarpour added enhancement New feature or request Launch labels Jul 29, 2024
@sahand-asgarpour sahand-asgarpour merged commit 1a812db into master Aug 9, 2024
2 of 4 checks passed
@sahand-asgarpour sahand-asgarpour deleted the feat/136-simple-graph-segmentation-for-damages branch August 9, 2024 09:20
@ArdtK
Copy link
Contributor

ArdtK commented Aug 13, 2024

@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
Copy link
Contributor Author

sahand-asgarpour commented Aug 13, 2024

@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 Carsopre restored the feat/136-simple-graph-segmentation-for-damages branch August 30, 2024 14:13
@Carsopre Carsopre deleted the feat/136-simple-graph-segmentation-for-damages branch August 30, 2024 14:16
@Carsopre
Copy link
Collaborator

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Simple graph segmentation for damages
4 participants