Skip to content

Conversation

dschult
Copy link
Contributor

@dschult dschult commented Jul 3, 2024

Fixes #21064

Some construction functions (like hstack and vstack) do not handle 1D input well for some formats. The shape was assumed to be 2D and not checked for 1D. This PR:

  • adds tests for these cases,
  • adds tests for better error messages in functions kron and kronsum when 1D input it used,
  • fixes ndim/shape related errors in _construct.py by using _shape_as_2d where needed

This might be too late, but if possible, it would be good to get this into 1.14.1. @tylerjereddy

@dschult dschult added defect A clear bug or issue that prevents SciPy from being installed or used as expected scipy.sparse backport-candidate This fix should be ported by a maintainer to previous SciPy versions. labels Jul 3, 2024
@dschult dschult requested a review from perimosocordiae as a code owner July 3, 2024 19:45
@tylerjereddy tylerjereddy added this to the 1.15.0 milestone Jul 3, 2024
@tylerjereddy
Copy link
Contributor

This might be too late, but if possible, it would be good to get this into 1.14.1.

Should be fine unless the review takes forever. I don't have a concrete timeline for 1.14.1 just yet, was hoping to do a little bit of coding/code review before jumping on another release right away.

@perimosocordiae
Copy link
Member

Thanks for fixing this, Dan. I left some optional tweaks to the kronsum error messages.

Co-authored-by: CJ Carey <perimosocordiae@gmail.com>
@perimosocordiae
Copy link
Member

Oops, didn't realize the test would fail:

    def test_kronsum_ndim_exceptions(self):
>       with pytest.raises(ValueError, match='is not 2D'):
E       AssertionError: Regex pattern did not match.
E        Regex: 'is not 2D'
E        Input: 'kronsum requires 2D inputs. `B` is 1D.'

@perimosocordiae perimosocordiae merged commit 06f3d20 into scipy:main Jul 8, 2024
@perimosocordiae
Copy link
Member

Merged! Thanks @dschult

@dschult dschult deleted the fix_hstack branch July 8, 2024 13:24
ev-br pushed a commit to ev-br/scipy that referenced this pull request Jul 14, 2024
…v-stacks/kron/kronsum (scipy#21108)

* fix 1D for vstack/hstack and Improve 1D err msg stacks & kron/kronsum

* improve error messages in kronsum

Co-authored-by: CJ Carey <perimosocordiae@gmail.com>

* adjust exception test match string

---------

Co-authored-by: CJ Carey <perimosocordiae@gmail.com>
@tylerjereddy tylerjereddy modified the milestones: 1.15.0, 1.14.1 Aug 11, 2024
tylerjereddy pushed a commit to tylerjereddy/scipy that referenced this pull request Aug 11, 2024
…v-stacks/kron/kronsum (scipy#21108)

* fix 1D for vstack/hstack and Improve 1D err msg stacks & kron/kronsum

* improve error messages in kronsum

Co-authored-by: CJ Carey <perimosocordiae@gmail.com>

* adjust exception test match string

---------

Co-authored-by: CJ Carey <perimosocordiae@gmail.com>
@tylerjereddy tylerjereddy mentioned this pull request Aug 11, 2024
5 tasks
@tylerjereddy tylerjereddy removed the backport-candidate This fix should be ported by a maintainer to previous SciPy versions. label Aug 20, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
defect A clear bug or issue that prevents SciPy from being installed or used as expected scipy.sparse
Projects
None yet
Development

Successfully merging this pull request may close these issues.

BUG: sparse: hstack/vstack between a sparse and ndarray breaks in >=1.13.0
3 participants