8000 ENH: add block() function to create block arrays by sotte · Pull Request #5057 · numpy/numpy · GitHub
[go: up one dir, main page]

Skip to content

ENH: add block() function to create block arrays #5057

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

Closed
wants to merge 7 commits into from

Conversation

sotte
Copy link
Contributor
@sotte sotte commented Sep 7, 2014

block is similar to matlab's "square bracket stacking" functionality: [A B; C D].

So you can write:

>>> A = np.array([[1, 2, 3]])
>>> B = np.array([[2, 3, 4, 5, 6, 7]])
>>> c = np.array([8, 3, 4, 5, 6, 7])
>>> d1 = np.array([9])
>>> d2 = np.array([8, 7, 6, 5, 4])
>>> block([[A, A],
           [B],
           [c],
           [d1, d2]])
array([[1, 2, 3, 1, 2, 3],
       [2, 3, 4, 5, 6, 7],
       [8, 3, 4, 5, 6, 7],
       [9, 8, 7, 6, 5, 4]])

The one thing I'm not super happy with is that 1-D arrays are treated as row vectors and I personally think of them as column vectors.

Are the doctests sufficient or should I create proper unittests?

@seberg
Copy link
Member
seberg commented Sep 8, 2014

Being as cynical as I am, how does this play together with all the other stacking functions ;)? What I mean is, can you write to the mailing list as usual, because it is a new function, plus I think we can probably find a better name.

Another thing I think, we limit the outer scope to list/tuple explicitly anyway (not much choice there I guess), so if we add it, maybe make it N-dimensional?

@seberg
Copy link
Member
seberg commented Sep 8, 2014

Oh, about doctests... Those are not run currently, so not sure if they are sufficient, but add them as normal tests too at least. (Since I like at least some failure tests often, there probably should be a few more tests anyway)

@argriffing
Copy link
Contributor

By the way, this looks similar to bmat.

@seberg
Copy link
Member
seberg commented Sep 8, 2014

How would anyone know, none of the devs use matrices ;)

@argriffing
Copy link
Contributor

none of the devs use matrices ;)

This would explain why numpy/scipy functions related to np.matrix are so buggy :)

@seberg
Copy link
Member
seberg commented Sep 8, 2014

I think that is mostly because of the intrinsic problems with matrices if you don't want to constantly check matrices explicitly... But enough for that.

@abalkin
Copy link
Contributor
abalkin commented Mar 23, 2015

The one thing I'm not super happy with is that 1-D arrays are treated as row vectors and I personally think of them as column vectors.

Why?

They sure look like rows when printed:

In [8]: np.arange(10)
Out[8]: array([0, 1, 2, 3, 4, 5, 6, 7, 8, 9])

@sotte
Copy link
Contributor Author
sotte commented May 10, 2015

I updated my PR. Because of the discussion here #5605 I renamed the function to block because you basically create a block matrix. I moved the tests into a proper TestCase.

Some remarks:

  • I also played with the implementation of bmat as skeleton for block. But the implementation is too complex for my liking. And block in the current form does everything I want.
  • On the mailinglist we discussed more advanced versions of block but I think the current version actually versatile enough.

Feedback welcome!

@sotte
Copy link
Contributor Author
sotte commented May 10, 2015

@abalkin it's just my personal pet I guess :) It does not really matter for the pull request.

@sotte sotte changed the title ENH: add stack() function. ENH: add block() function to create block arrays May 10, 2015
@sotte
Copy link
Contributor Author
sotte commented May 14, 2015

Do you think this is ready to be merged?

`block` is similar to Matlab's square bracket stacking functionality for block
matices. `block` is an addition to the current stacking functions vstack,
hstack, stack.
@sotte
Copy link
Contributor Author
sotte commented Jan 9, 2016

One of my new year's resolution is to get my pull requests merged. So, what do you think?

@rgommers rgommers added this to the 1.12.0 release milestone Jan 18, 2016
@rgommers
Copy link
Member

One of my new year's resolution is to get my pull requests merged. So, what do you think?

Sorry that this didn't get reviewed yet Stefan. It seems useful to multiple people and isn't hard to review, so let's make sure we have a serious look before the next release (I gave it the 1.12.0 milestone).

"""
Create block arrays similar to Matlab's "square bracket stacking":

[A A; B B]
Copy link
Member

Choose a reason for hiding this comment

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

Can you hide this somewhere in the Notes section? Putting Matlab syntax here so prominently feels wrong, plus for a minute I read this as the Python syntax to use.

@sotte
Copy link
Contributor Author
sotte commented Feb 4, 2016

I can't see the forest for the trees so I talked to my rubber duck.

What do we expect? Do we want block([A, B]) to be equivalent to vstack([A, B]), i.e., should A and B be stacked in a row? Of course, right? But what about block([A, [B, C]]) then? On first glance this looks like the first example. Do we have to analyse the complete structure of the nested arrays? Should we throw an Exception there? Maybe we should enforce explicit brackets?
block([[A, B]]) and block([[A], [B, C]] are clear, but they are verbose.

If we use *args it's less verbose block(A, [B, C]) and I think it's also clear that block([A, B]) is equivalent to vstack([A, B]). (Also, when using *args the implementation is clean because *args is always a tuple where each element represents a row.)

Now, I'm actually inclined to use *args despite my general dislike of it.

@charris
Copy link
Member
charris commented Jun 14, 2016

@sotte Take a look at the commit message guidelines: doc/source/dev/gitwash/development_workflow.rst. You also need to squash your commits and redo the message(s), you can use git rebase -i HEAD~7 for that followed by a force push git push -f origin HEAD.

@charris
Copy link
Member
charris commented Jun 21, 2016

Updated in #7768, so closing this. Thanks @sotte .

@charris charris closed this Jun 21, 2016
@sotte
Copy link
Contributor Author
sotte commented Jun 21, 2016

@charris I guess I'm too late. Thanks a lot! Was a lot of fun...as always!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants
0