Skip to content
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

Improvement: Bound networkx to less than 3.3 #66

Conversation

andrew-causalens
Copy link
Contributor

@andrew-causalens andrew-causalens commented Apr 10, 2024

Description and Motivation

networkx 3.3 (release notes) claims to fix a bug (PR) on simple paths but it actually returns paths of length one when source and destination is the same. This is suboptimal behavior particularly if there is no self loop edge.

Additional Context

We will add 3.3 support in future but first need to understand impact on our other packages. Plus our base image for 2024 Q2 is using networkx 3.2.1.

How Has This Been Tested?

Ensure we are using networkx 3.2.1 in unit tests for python 3.11. The actions on this PR do that.

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Improvement (non-breaking change which improves functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Refactor (moving code around, general refactoring improvements)
  • Documentation (documentation)

PR Checklist:

  • I have reviewed my own PR? (review the PR as you are reviewing someone else's PR)
  • I have implemented all requirements? (see JIRA, project documentation)
  • I am affecting someone else's work? If so: I have added those people as reviewers?
  • I have added comments to all the bits that are hard to follow
  • At a bare minimum, I tested this manually
  • I have added unit test(s)
  • Is this a bugfix? If so have I added a regression test?
  • Does this PR satisfy the one PR, one concern rule?
  • If needed, documentation has been added/updated?
  • I have updated the appropriate changelog with a line for my changes
  • If this PR breaks reproducibility, have I added a note in the changelog explaining the break in reproducibility
    and its extent?

Screenshots (if appropriate):

@andrew-causalens andrew-causalens added the In progress This PR is still work in progress and should not be reviewed yet. label Apr 10, 2024
@andrew-causalens andrew-causalens self-assigned this Apr 10, 2024
@andrew-causalens andrew-causalens requested a review from a team as a code owner April 10, 2024 09:09
@andrew-causalens andrew-causalens changed the title Improvement: Update to support change in path behavior in networkx 3.3 Improvement: Bound networkx to less than 3.3 Apr 10, 2024
@andrew-causalens andrew-causalens requested review from a team as code owners April 10, 2024 09:19
@andrew-causalens andrew-causalens added Waiting for review This PR is now ready for the review process, this means tests passed and all deliverables are in. and removed In progress This PR is still work in progress and should not be reviewed yet. labels Apr 10, 2024
Copy link
Contributor

@maxelliott-causalens maxelliott-causalens left a comment

Choose a reason for hiding this comment

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

changelog?

Copy link
Contributor

@maxelliott-causalens maxelliott-causalens left a comment

Choose a reason for hiding this comment

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

🙏

@andrew-causalens andrew-causalens merged commit b9754d1 into main Apr 10, 2024
1 check passed
@andrew-causalens andrew-causalens deleted the CAUSALAI-4683-update-causal-paths-given-change-to-networkx branch April 10, 2024 09:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Waiting for review This PR is now ready for the review process, this means tests passed and all deliverables are in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants