iframe-proxy

andrei-papou · 2019-10-01T13:05:43Z

Issue: #667

Renamed stack to concatenate to follow the behaviour of numpy and also added stack function. Examples:

concatenate

use ndarray::{arr2, Axis, concatenate};

let a = arr2(&[[2., 2.], [3., 3.]]);
assert!(
     concatenate(Axis(0), &[a.view(), a.view()])
     == Ok(arr2(&[[2., 2.], [3., 3.], [2., 2.], [3., 3.]]))
);

stack

use ndarray::{arr2, arr3, stack, Axis};

let a = arr2(&[[2., 2.],
               [3., 3.]]);
assert!(
    stack![Axis(0), a, a]
    == arr3(&[[[2., 2.],
               [3., 3.]],
              [[2., 2.],
               [3., 3.]]])
);

Merge main rep master to fork master

Update fork master

Update master

Up to date with the main repo master

bluss · 2019-10-01T13:36:06Z

bluss · 2019-10-02T05:45:53Z

+    }
+    let arrays: Vec<ArrayView<A, D::Larger>> =
+        arrays.into_iter().map(|a| a.insert_axis(axis)).collect();
+    concatenate(axis, &arrays)


Implementation strategy wise, can't we take the old stack, put that into an internal function. Give it a parameter that tells it whether to remove an axis or not. It can be a type parameter, since it influences the return type (the dim of the returned array).

That way both stack and concatenate can use this internal function and we don't need the intermediate vector here in concatenate

Looks good! I'll try to implement it this way.

bluss · 2019-10-02T07:18:38Z

This branch will eventually need to be rebased to skip the merges in the history (see commit view). The blast back to #1, #5, #8 etc in ndarray history is fun, but we don't want to have those merges there.

Also, "this is the right direction" isn't supposed to sound like "nice try". It's supposed to say.. This is the right destination, this is how we want to name those functions. To be pragmatic I think we'd like to have this in 0.13.x, as a non-breaking change in the first round (preparing to put the functions in their right places in the next version).

function

- got rid of the redundant allocation in `stack_new_axis` implementation - renamed the functions and macros to make the change backward compatible: `concatenate` -> `stack`, `stack` -> `stack_new_axis`

Merge main repo master to the fork

andrei-papou · 2019-10-17T00:14:45Z

@bluss sorry for the delay, the month is pretty busy. I've refactored the new function to get rid of the redundant allocation. Also I renamed the functions in the backward compatible way. Thanks.

LukeMathWalker · 2019-12-15T19:37:09Z

 /// [1]: fn.stack.html
 ///
-/// ***Panics*** if the `stack` function would return an error.
+/// ***Panics*** if the `concatenate` function would return an error.


concatenate does not exist yet, we shouldn't refer to it here.

LukeMathWalker · 2019-12-15T19:41:18Z

If we want to introduce this in 0.13.x, we should probably:

mark the existing stack function as deprecated, suggesting to use stack_new_axis;
introduce concatenate and concatenate!, which underneath are just calling stack for now;
introduce stack_new_axis which will become the "new" stack.

What do you think? @andrei-papou
Always difficult to change what a name means :/

andrei-papou · 2019-12-16T14:25:29Z

mark the existing stack function as deprecated, suggesting to use stack_new_axis;

@LukeMathWalker shouldn't we suggest to use concatenate instead? concatenate is a new name for the current stack function while stack_new_axis behaves in a different way.

LukeMathWalker · 2019-12-16T14:27:56Z

Yes, you are right 😅

- introduced concatenate function - deprecated stack function since 0.13.0

- updated deprecation version for stack function - suppressed deprecation warnings

Main repo master to fork master

…nd-concatenate

andrei-papou · 2019-12-18T19:46:23Z

@LukeMathWalker ready, but I had to suppress a few deprecation warnings: we still need stack function in concatenate implementation, in lib.rs and in tests.

andrei-papou · 2020-03-30T07:49:15Z

@bluss @LukeMathWalker could you please do another round of review? Looks like all the comments have been resolved quite a while ago. Thanks.

bluss · 2020-04-18T11:16:18Z

+
+pub fn stack_new_axis<A, D>(
+    axis: Axis,
+    arrays: Vec<ArrayView<A, D>>,


stack_new_axis and concatenate should agree on if they take the arrays as a slice or a Vec, they should have the same interface if they can. Avoiding cloning of the array views seems like a minor issue, so it can be decided either way (slice or vec)

bluss · 2020-04-18T11:16:40Z

+    stack(axis, arrays)
+}
+
+pub fn stack_new_axis<A, D>(


Missing doc comment :)

bluss · 2020-04-18T11:17:06Z

@@ -109,3 +182,71 @@ macro_rules! stack {
        $crate::stack($axis, &[ $($crate::ArrayView::from(&$array) ),* ]).unwrap()


The stack macro should also be deprecated

bluss · 2020-04-18T11:18:10Z

+    res_dim.set_axis(axis, arrays.len());
+
+    // we can safely use uninitialized values here because they are Copy
+    // and we will only ever write to them


This looks good, it follows what we do in stack. We'll revise handling of uninit data, but it's not in scope for this PR. (#796 )

bluss · 2020-04-18T11:19:18Z

+    let mut res = Array::from_shape_vec(res_dim, v)?;
+
+    res.axis_iter_mut(axis)
+        .zip(arrays.into_iter())


this zip seems to show that we would be fine with just using a slice of ArrayView as input, we don't need to use them by value

bluss · 2020-04-20T19:23:57Z

 macro_rules! s(
    // convert a..b;c into @convert(a..b, c), final item
-    (@parse $dim:expr, [$($stack:tt)*] $r:expr;$s:expr) => {
+    (@parse $dim:expr, [$($concatenate:tt)*] $r:expr;$s:expr) => {


This rename of the variable inside the macro is an incidental mistake, and should not be in the PR

Oh I removed this in my rebase-fix of the branch. No other changes in that branch. :)

bluss · 2020-04-21T14:30:52Z

Andrew and others added 4 commits April 3, 2019 11:54

Merge pull request #1 from rust-ndarray/master

b39a0bb

Merge main rep master to fork master

Merge pull request #5 from rust-ndarray/master

efd686d

Update fork master

Merge pull request #8 from rust-ndarray/master

8cd5d5a

Update master

Merge pull request #9 from rust-ndarray/master

0e9d65d

Up to date with the main repo master

bluss reviewed Oct 2, 2019

View reviewed changes

andrei-papou added 5 commits October 2, 2019 11:02

Renamed stack to concatenate and added numpy-compliant stack

6525f6e

function

Rustfmt fixes

2d04320

Commit changelog:

ced7ed9

- got rid of the redundant allocation in `stack_new_axis` implementation - renamed the functions and macros to make the change backward compatible: `concatenate` -> `stack`, `stack` -> `stack_new_axis`

Merge pull request #10 from rust-ndarray/master

443fa86

Merge main repo master to the fork

Rustfmt fixes

68f0e95

andrei-papou mentioned this pull request Dec 13, 2019

Add dstack, vstack, and hstack #667

Closed

LukeMathWalker reviewed Dec 15, 2019

View reviewed changes

andrei.papou added 4 commits December 18, 2019 21:54

Changelog:

d997c18

- introduced concatenate function - deprecated stack function since 0.13.0

Changelog:

8e12f82

- updated deprecation version for stack function - suppressed deprecation warnings

Merge pull request #11 from rust-ndarray/master

ef3d5b6

Main repo master to fork master

Merge branch 'master' of github.com:andrei-papou/ndarray into stack-a…

1e91fe4

…nd-concatenate

bluss self-assigned this Apr 16, 2020

bluss mentioned this pull request Apr 18, 2020

Support Clone elements in stack and select #269

Closed

bluss reviewed Apr 18, 2020

View reviewed changes

Comment thread src/stacking.rs

stack(axis, arrays)

}

pub fn stack_new_axis<A, D>(

bluss Apr 18, 2020

Copy link
Copy Markdown

Member

Choose a reason for hiding this comment

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

Missing doc comment :)

bluss reviewed Apr 18, 2020

View reviewed changes

bluss reviewed Apr 20, 2020

View reviewed changes

andrei-papou mentioned this pull request Sep 25, 2020

Stack and concatenate #844

Merged

andrei-papou closed this Oct 14, 2020

		@@ -109,3 +182,71 @@ macro_rules! stack {
		$crate::stack($axis, &[ $($crate::ArrayView::from(&$array) ),* ]).unwrap()

Sunbelt Computer Software

PL/B Language Development and Support

Uh oh!

Conversation

andrei-papou commented Oct 1, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

concatenate

stack

Uh oh!

bluss commented Oct 1, 2019

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

bluss commented Oct 2, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

andrei-papou commented Oct 17, 2019

Uh oh!

Choose a reason for hiding this comment

Uh oh!

LukeMathWalker commented Dec 15, 2019

Uh oh!

andrei-papou commented Dec 16, 2019

Uh oh!

LukeMathWalker commented Dec 16, 2019

Uh oh!

andrei-papou commented Dec 18, 2019

Uh oh!

andrei-papou commented Mar 30, 2020

Uh oh!

bluss Apr 18, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

bluss commented Apr 21, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

andrei-papou commented Oct 1, 2019 •

edited

Loading

bluss commented Oct 2, 2019 •

edited

Loading

bluss Apr 18, 2020 •

edited

Loading