causalnex icon indicating copy to clipboard operation
causalnex copied to clipboard

Fix/issue#146 set cpd for parentless nodes

Open Rishab26 opened this issue 2 years ago • 1 comments

Why was this PR created?

This PR is created to fix the issue in set_cpd method. set_cpd currently works when df_node.columns has type pd.MultiIndex. This is because nodes having one or more parents are built in this form. This fix resolves the Error shown in #146 which has a case where the node "paid" does not have a parent node.

paid  
0 0.938567
1 0.061433

I fix this by checking if the df.columns is MultiIndex and passing an empty dictionary if not since this indicates it does not have any parent node.

image

How has this been tested?

I have tested the issue with the dataset provided in #146 and it works fine now with this fix. I have also checked this with other local datasets and it has successfully loaded using set_cpd. Dataset and test_code added in tests.

Checklist

  • [x] Read the contributing guidelines
  • [ Not Applicable ] Opened this PR as a 'Draft Pull Request' if it is work-in-progress
  • [x] Updated the documentation to reflect the code changes
  • [x] Added a description of this change and added my name to the list of supporting contributions in the RELEASE.md file
  • [x] Added tests to cover my changes
  • [ ] Assigned myself to the PR

Notice

  • [x] I acknowledge and agree that, by checking this box and clicking "Submit Pull Request":

  • I submit this contribution under the Apache 2.0 license and represent that I am entitled to do so on behalf of myself, my employer, or relevant third parties, as applicable.

  • I certify that (a) this contribution is my original creation and / or (b) to the extent it is not my original creation, I am authorised to submit this contribution on behalf of the original creator(s) or their licensees.

  • I certify that the use of this contribution as authorised by the Apache 2.0 license does not violate the intellectual property rights of anyone else.

Rishab26 avatar Mar 31 '22 00:03 Rishab26

@qbphilip @oentaryorj Is there anything else required for this PR before the merge?

Rishab26 avatar Jul 23 '22 21:07 Rishab26

@qbphilip @oentaryorj Is there anything else required for this PR before the merge?

Thanks for the PR. We have made further changes on the test cases, i.e., instead of putting it in a new test script, we added a new pytest.fixture and a new test case for parentless node in tests/test_bayesiannetwork.py

oentaryorj avatar Aug 26 '22 06:08 oentaryorj

thanks a lot @Rishab26 ! Amazing contribution, thanks for the bug fix!