Skip to content

ENH: Require that arguments to block lie on a grid#2

Open
eric-wieser wants to merge 1 commit intonp-blockfrom
np-block-grid
Open

ENH: Require that arguments to block lie on a grid#2
eric-wieser wants to merge 1 commit intonp-blockfrom
np-block-grid

Conversation

@eric-wieser
Copy link
Owner

@eric-wieser eric-wieser commented Apr 6, 2017

This represents the largest use case, so asking for more flexibility should
be done explicitly.

Builds on numpy#8886.

This seems to add a 30% overhead for small (25ish) arrays

@eric-wieser
Copy link
Owner Author

eric-wieser commented Apr 6, 2017

@madphysicist:

What is the use case for the proposed grid parameter?

To by default prevent users from making mistakes. Almost all the uses that I've seen of block matrices are trying to assemble something of the form:

A A|B B B
A A|B B B
---+-----
C C|D D D
C C|D D D
C C|D D D

So I think we should raise an error if people try to construct the following accidentally

A A|B B B
A A|B B B
---+-+---
C C C|D D
C C C|D D
C C C|D D

@madphysicist
Copy link

I am a big fan of letting people make their own mistakes, so -1 on this feature. But don't let that stop you if the consensus is in favor.

@madphysicist
Copy link

On second thought, if it's disabled by default and therefore unlikely to cause surprises, count me as neutral, despite the added maintenance burden.

@charris
Copy link

charris commented Apr 12, 2017

I lean towards @madphysicist on this as I suspect most user errors would still show up even without the added check because it takes some effort to put together a valid non-gridded array. OTOH, if the default is False, then if the user sets it to True, it would be a nice marker of intent and serve as a form of code documentation.

This represents the largest use case, so asking for more flexibility should
be done explicitly
@mhvk
Copy link

mhvk commented Apr 20, 2017

I'd also favour a default of False.

@eric-wieser
Copy link
Owner Author

I guess as a future path, np.block(non_grid) could warn, np.block(non_grid, grid=False) would be silent, and np.block(non_grid, grid=True) could error.

So in fact, this doesn't need to be introduced in the first pass

@eric-wieser
Copy link
Owner Author

it takes some effort to put together a valid non-gridded array

Simply writing block([a, b], [d, c]) instead of block([[a, b], [c, d]]) would be a way to do this accidentally - but I suppose this class of bug is generally uncatchable anyway

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.

4 participants