Skip to content

Conversation

@frgoe003
Copy link
Contributor

@frgoe003 frgoe003 commented Feb 7, 2025

No description provided.

pred_links["system_id"] = pred_links["reference_system_id"]
assert pred_links is not None, "pred_links is None"
num_pred_links = pred_links.groupby("query_system").size()
num_pred_links = pred_links.groupby("system_id").size()
Copy link
Member

Choose a reason for hiding this comment

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

@frgoe003 , the pred_links df is not used later - thus assigning system_id seems redundant.
Is there an issue with using query_system variable?

If so, them maybe doing this would be less confusing?

num_pred_links = pred_links.groupby("reference_system_id").size()

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The query_system column is not present in the df. I agree, was just following the apo_links structure. The best way would be to change both groupby operations to reference_system_id.

num_apo_links = apo_links.groupby("reference_system_id").size()
num_pred_links = pred_links.groupby("reference_system_id").size()

Copy link
Member

Choose a reason for hiding this comment

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

yes, thanks! Please make both reference_system_id and then I think we can merge!

Copy link
Member

Choose a reason for hiding this comment

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

Thanks, @frgoe003 !
This should work. I noted that one test failed, but that is related with networkit version update. I am dealing with this on another PR #98 - once I verify the fix - will rebase this PR on it and merge. So, no worries!

Copy link
Member

@OleinikovasV OleinikovasV left a comment

Choose a reason for hiding this comment

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

LG!

@OleinikovasV OleinikovasV merged commit 6ce4572 into plinder-org:main Feb 17, 2025
3 checks passed
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.

2 participants