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

added future warning to directed_combinatorial_laplacian for new return type #4142

Closed
wants to merge 1 commit into from

Conversation

willpeppo
Copy link
Contributor

#4141 implements the necessary change.

@@ -272,6 +273,10 @@ def directed_combinatorial_laplacian_matrix(
"""
from scipy.sparse import spdiags, linalg

warnings.warn(
Copy link
Member

Choose a reason for hiding this comment

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

Don't we still need to figure out whether we are using scipy.sparse, pysparse, or scipy.csgraph first?

Copy link
Member

Choose a reason for hiding this comment

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

Yes, we do -- but we were working through one function for this in the sprint and decided to make them consistent first and then once we have decided which way to proceed we could update it. Also, we might want to eventually make the type controllable too...???... Do we?

We actually had some tricky parts today due to scipy.sparse matrices turning into numpy matrices when we didn't expect them to. That could lead to memory bottlenecks.. and unexpected types too.

@jarrodmillman jarrodmillman marked this pull request as draft August 7, 2020 20:32
Base automatically changed from master to main March 4, 2021 18:20
@rossbar
Copy link
Contributor

rossbar commented Jan 27, 2022

I'm closing this as it's been superseded in the transition to the scipy.sparse array interface #5139

@rossbar rossbar closed this Jan 27, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants