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

Fixed NetworkX 3 compatibility and switched to sparse arrays (not matrices) #1018

Merged
merged 10 commits into from
Mar 27, 2023

Conversation

bdpedigo
Copy link
Collaborator

@bdpedigo bdpedigo commented Jan 12, 2023

  • Does this PR have a descriptive title that could go in our release notes?
  • Does this PR add any new dependencies?
  • Does this PR modify any existing APIs?
    • Is the change to the API backwards compatible?
      • no
  • Have you built the documentation (reference and/or tutorial) and verified the generated documentation is appropriate?

Reference Issues/PRs

Fixes #961
Proper fix of #1015

What does this implement/fix? Briefly explain your changes.

Any other comments?

  • Technically, we're losing support for nx.OrderedGraph, but I'm not losing and sleep over that one. I was even considering just switching to NetworkX 3 support only but I don't think it's fully necessary.

@bdpedigo bdpedigo changed the title Fixed NetworkX 3 compatibility Fixed NetworkX 3 compatibility and switched to sparse arrays Mar 24, 2023
@bdpedigo bdpedigo marked this pull request as ready for review March 24, 2023 22:52
@bdpedigo bdpedigo requested review from ebridge2 and j1c March 24, 2023 23:01
@bdpedigo bdpedigo changed the title Fixed NetworkX 3 compatibility and switched to sparse arrays Fixed NetworkX 3 compatibility and switched to sparse arrays (not matrices) Mar 24, 2023
Copy link
Contributor

@tathey1 tathey1 left a comment

Choose a reason for hiding this comment

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

There was one line I didn't immediately understand.

Out of curiosity, how did you find and iterate through all the instances of "csr_matrix"?

graspologic/pipeline/embed/_types.py Show resolved Hide resolved
graspologic/utils/utils.py Show resolved Hide resolved
@bdpedigo
Copy link
Collaborator Author

bdpedigo commented Mar 27, 2023

Out of curiosity, how did you find and iterate through all the instances of "csr_matrix"?

@tathey1 vs code has a "rename symbol" command that can do it all in one go

Copy link
Contributor

@tathey1 tathey1 left a comment

Choose a reason for hiding this comment

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

mostly changing to csr_array, and updating a couple method names.

@bdpedigo bdpedigo merged commit 13d0d46 into dev Mar 27, 2023
@bdpedigo bdpedigo deleted the networkx3 branch March 27, 2023 17:39
bdpedigo added a commit that referenced this pull request May 22, 2023
…rices) (#1018)

* Update setup.cfg

* add conditional types logic for pipeline

* fix a reference to sparse matrix

* remove all reference to csr_matrix

* fix array

* remove nx.testing

* fix graphs equal

* fix some weird reversions to csr_matrix

* remove OrderedGraph support

* fix tutorial reference to csr_array

---------

Co-authored-by: hugwuoke <85888975+hugwuoke@users.noreply.github.com>
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.

Update package to use new scipy sparse *array* standard
3 participants