Skip to content

Add sparse matrix support for some matrix plotting functions#794

Merged
bdpedigo merged 6 commits into
devfrom
sparse-matrix-plot
Jun 7, 2021
Merged

Add sparse matrix support for some matrix plotting functions#794
bdpedigo merged 6 commits into
devfrom
sparse-matrix-plot

Conversation

@bdpedigo
Copy link
Copy Markdown
Collaborator

@bdpedigo bdpedigo commented Jun 3, 2021

  • Does this PR add any new dependencies?
  • Does this PR modify any existing APIs?
    • Is the change to the API backwards compatible?
  • Have you built the documentation (reference and/or tutorial) and verified the generated documentation is appropriate?

Reference Issues/PRs

None

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

  • Allows the user to pass sparse matrices to adjplot and matrixplot when using the scattermap option

Any other comments?

  • Could absolutely have a conversation about the best API for these functions. I'm not currently that happy with it, but don't want to let that hold up this PR.

@netlify
Copy link
Copy Markdown

netlify Bot commented Jun 3, 2021

❌ Deploy Preview for graspologic failed.

🔨 Explore the source changes: eac4410

🔍 Inspect the deploy log: https://app.netlify.com/sites/graspologic/deploys/60b94aeef8fa290008e288fb

@bdpedigo
Copy link
Copy Markdown
Collaborator Author

bdpedigo commented Jun 3, 2021

TODO:

  • Update documentation
  • Add a test

@bdpedigo bdpedigo marked this pull request as ready for review June 3, 2021 21:00
@bdpedigo bdpedigo requested a review from nicaurvi June 3, 2021 21:16
@bdpedigo
Copy link
Copy Markdown
Collaborator Author

bdpedigo commented Jun 3, 2021

@nyecarr any chance you'd be willing to have a look at this since we were just talking about it?

@bdpedigo
Copy link
Copy Markdown
Collaborator Author

bdpedigo commented Jun 7, 2021

@asaadeldin11 any chance you could take a quick look at this? No worries if not just lmk

@bdpedigo bdpedigo requested a review from asaadeldin11 June 7, 2021 17:06
Copy link
Copy Markdown
Contributor

@asaadeldin11 asaadeldin11 left a comment

Choose a reason for hiding this comment

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

Looks good to me @bdpedigo .

@bdpedigo bdpedigo merged commit 541821f into dev Jun 7, 2021
@bdpedigo bdpedigo deleted the sparse-matrix-plot branch June 7, 2021 17:22
@bdpedigo
Copy link
Copy Markdown
Collaborator Author

bdpedigo commented Jun 7, 2021

thanks @asaadeldin11 !

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.

2 participants