Skip to content

Conversation

@lucascolley
Copy link
Contributor

@lucascolley lucascolley commented Jan 27, 2026

What type of PR is this? (check all applicable)

  • πŸ’Ύ Refactor
  • πŸͺ„ Feature
  • 🐞 Bug Fix
  • πŸ”§ Optimization
  • πŸ“š Documentation
  • πŸ§ͺ Test
  • πŸ› οΈ Other

Related issues

  • Related issue #
  • Closes #

Checklist

  • Code follows style guide
  • Tests added
  • Documented the changes

Please explain your changes below.

Comment on lines -454 to +455
coords = np.empty((2, x.nnz), dtype=x.row.dtype)
coords[0, :] = x.row
coords[1, :] = x.col
return COO(
coords,
x.coords,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

maybe x.coords doesn't exist for coo_matrix?

Copy link
Collaborator

Choose a reason for hiding this comment

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

It doesn't, indeed. I think we might need to specialize for coo_matrix vs coo_array.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Something like:

def coords(x):
    c = getattr(x, "coords", None)
    if c is None:
        c = (x.row, x.col)
    return c

Comment on lines -1194 to +1188
result = scipy.sparse.coo_matrix((self.data, (self.coords[0], self.coords[1])), shape=self.shape)
result = scipy.sparse.coo_array((self.data, self.coords), shape=self.shape)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

obviously this is a breaking change, this diff is just for demonstration

Copy link
Collaborator

Choose a reason for hiding this comment

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

I'm willing to accept this breakage given that *_array is now recommended over *_matrix.

In fact, I'd change it in all formats, not just this one, for consistency.

@codspeed-hq
Copy link

codspeed-hq bot commented Jan 28, 2026

Merging this PR will not alter performance

βœ… 340 untouched benchmarks


Comparing lucascolley:nd-scipy-coo (3034e87) with main (67be471)

Open in CodSpeed

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