Miscellaneous improvements regarding network splitting and simplification #268
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## dev #268 +/- ##
==========================================
+ Coverage 80.89% 80.92% +0.02%
==========================================
Files 16 16
Lines 5036 5043 +7
==========================================
+ Hits 4074 4081 +7
Misses 962 962 ☔ View full report in Codecov by Sentry. |
Works towards se-sic#260. Signed-off-by: Maximilian Löffler <[email protected]>
bockthom
reviewed
Sep 6, 2024
This change is in-line with the current treatment of the elements that occur after the last bin. Further, this fixes a minor "bug" where bin indecees are used to determine bin labels by indexing into a list of labels. Elements before the first bin received bin index 0, however, R is 1-indexed. Note: This "bug" did not cause problems because the bin with the incorrect label will be ignored later anyways. Works towards se-sic#267. Signed-off-by: Maximilian Löffler <[email protected]>
Add a function that takes a network as input and simply removes all edges that are exact duplicates of each other. Works towards se-sic#138. Signed-off-by: Maximilian Löffler <[email protected]>
Works towards se-sic#138. Signed-off-by: Maximilian Löffler <[email protected]>
Works towards se-sic#265. Signed-off-by: Maximilian Löffler <[email protected]>
Works towards se-sic#265. Signed-off-by: Maximilian Löffler <[email protected]>
bockthom
requested changes
Sep 12, 2024
Signed-off-by: Maximilian Löffler <[email protected]>
Signed-off-by: Maximilian Löffler <[email protected]>
|
Just one final question before I can approve this PR: The splitting fix regarding the elements before the first bin does not have any user-visible effects? If this really is the case, I am fine with not mentioning it. If it can have user-visible effects, we should mention it in the NEWS.md. |
|
Yup, it has no user-visible effect. That is why I found it hard to justify mentioning it in the NEWS and decided against it 😄 |
bockthom
approved these changes
Sep 12, 2024
Merged
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Prerequisites
showcase.Rwith respect to my changes.dev.Description
This PR Includes somewhat minor fixes for multiple open issues.
Changelog
igraph::is.loopbyigraph::which_loopandigraph::list.graph.attributesbyigraph::graph_attr_namescumulativeparameter to theconstruct.rangesfunction which allows to create cumulative ranges from the given revisions