-
Notifications
You must be signed in to change notification settings - Fork 15
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
Additional Core/Peripheral Classification Methods #276
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I had a quick look at the implementation (but not yet at the tests).
Please find my initial comments below.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've had a quick look at your additions to the README, see my detailed comments below.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As I haven't had a look at the tests yet, I have now done so. I only have a few minor comments (but I did not check the actual centrality values; especially for eccentricity, I still don't have an impression whether the values do make sense or not 🙈)
In addition to my comments below, please don't forget about the changes to the README regarding "author networks" that we have agreed on in today's meeting.
Base implementation for new classification metrics. Documentation and testing still missing. Signed-off-by: Leo Sendelbach <[email protected]>
Tests use already existing network, this test cases are quite small. Additional research into potential rounding errors may be required. Signed-off-by: Leo Sendelbach <[email protected]>
Add default documentation, same as for already existing classification methods Signed-off-by: Leo Sendelbach <[email protected]>
add new entry under 'unversioned" Signed-off-by: Leo Sendelbach <[email protected]>
Also minor fixes as requested in PR Signed-off-by: Leo Sendelbach <[email protected]>
Changed ordering in readme Signed-off-by: Leo Sendelbach <[email protected]>
tests hierarchy with network previously used for another unit test Signed-off-by: Leo Sendelbach <[email protected]>
formulation change as requested Signed-off-by: Leo Sendelbach <[email protected]>
NEWS.md now includes update to README Signed-off-by: Leo Sendelbach <[email protected]>
expanded descripotion of network-based metrics to make clear that they can be used on any type of network Signed-off-by: Leo Sendelbach <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just two comments regarding wording:
also changed some wording issues in NEWS and README Signed-off-by: Leo Sendelbach <[email protected]>
also corrected copyright headers Signed-off-by: Leo Sendelbach <[email protected]>
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## dev #276 +/- ##
==========================================
+ Coverage 81.13% 81.67% +0.54%
==========================================
Files 16 16
Lines 5105 5147 +42
==========================================
+ Hits 4142 4204 +62
+ Misses 963 943 -20 ☔ View full report in Codecov by Sentry. |
Changed comment explaining potential issues with hierarchy classification Signed-off-by: Leo Sendelbach <[email protected]>
Putting the verb before the adverb Signed-off-by: Leo Sendelbach <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
First of all thank you for the enhancement @Leo-Send!
I have just found some small typos and wording issues. Once these are fixed, we can merge this PR.
implemented feedback on PR Signed-off-by: Leo Sendelbach <[email protected]>
Prerequisites
showcase.R
with respect to my changes.dev
.Description
Add four new metric which can be used for the classification of authors into core and peripheral:
Betweenness, which measures the number of shortest paths between developers that go through a given developer vertex;
Closeness, which measures how close a developer is to all others by taking the inverse of the sum of all of it's shortest paths;
Pagerank, which is based on Google's Pagerank algorithm, which is closely related to Eigenvector Centrality;
Eccentricity, which measures the distance to the furthest developer vertex.
Changelog
Added
Changed/Improved
README.md
(PR Additional Core/Peripheral Classification Methods #276, 6101e11, c6744c0, 5fc2da5)