Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

hetio.permute.permute_pair_list modifies pair_list in place #28

Closed
zietzm opened this issue Dec 3, 2018 · 1 comment
Closed

hetio.permute.permute_pair_list modifies pair_list in place #28

zietzm opened this issue Dec 3, 2018 · 1 comment

Comments

@zietzm
Copy link
Member

zietzm commented Dec 3, 2018

I recently discovered that permute_pair_list modifies the pair list it is given, in place. The returned pair list is then equivalent to the pair list that was passed as an argument to the function itself. I had not expected this behavior. There is no bug in the code that causes this. I am simply wondering if this is in fact the function's desired behavior. I could imagine that one wouldn't always want to modify an edge list in place.

image

@dhimmel
Copy link
Member

dhimmel commented Dec 3, 2018

I agree this is dangerous.

I searched GitHub for permute_pair_list, which returned 12 code files (all related to or using hetio). Most files are based on this notebook from Project Rephetio. Cell 5 defines treatments as a list of node pairs:

treatments = list(zip(treatment_df.compound_id, treatment_df.disease_id))

The permutation shuffles treatments in place. Cell 26 appears to assume treatments hasn't been permuted. So yes, this dangerous behavior may have caused some downstream damage. Specifically, the empiric facets of the degree-grouped edge existence probability plots here may be incorrect.

I think it makes sense for permute_pair_list to initially copy the list, unless in_place=True is specified. @zietzm want to make the PR?

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants