ENH added arange(), zeros(), ones() #46

Merged
merged 6 commits into from Feb 7, 2012

Conversation

Projects
None yet
3 participants
@stroxler

stroxler commented Feb 7, 2012

Closes #15

la/flarry.py
@@ -649,6 +650,279 @@ def intersection(axis, *args):
rc = list(rc)
rc.sort()
return rc
+# Instantiation shortcuts---------------------------------------------------

This comment has been minimized.

@kwgoodman

kwgoodman Feb 7, 2012

Owner

Move this section to the bottom of the file to keep alignment and binary ops together.

@kwgoodman

kwgoodman Feb 7, 2012

Owner

Move this section to the bottom of the file to keep alignment and binary ops together.

la/flarry.py
+
+def ones(shape=None, label=None, **kwargs):
+ """
+ Array of sequential integers, filled where later axes have smaller spacing.

This comment has been minimized.

@kwgoodman

kwgoodman Feb 7, 2012

Owner

Looks like a copy and paste error here. Copy first line of np.ones and replace array with larry.

@kwgoodman

kwgoodman Feb 7, 2012

Owner

Looks like a copy and paste error here. Copy first line of np.ones and replace array with larry.

la/flarry.py
+ data *= step; data += start
+ return larry(data, label)
+
+def ones(shape=None, label=None, **kwargs):

This comment has been minimized.

@kwgoodman

kwgoodman Feb 7, 2012

Owner

kwargs are confusing so we should avoid then whenever possible. Can we use the following (copied from np.ones) for the signature?

ones(shape=None, label=None, dtype=None, order='C')

Copy and paste the np.ones docstring and change what is necessary.

Same goes for zeros.

@kwgoodman

kwgoodman Feb 7, 2012

Owner

kwargs are confusing so we should avoid then whenever possible. Can we use the following (copied from np.ones) for the signature?

ones(shape=None, label=None, dtype=None, order='C')

Copy and paste the np.ones docstring and change what is necessary.

Same goes for zeros.

la/flarry.py
+ --------
+ A basic, 1d arange using the 'dtype' argument:
+
+ >>> la.ones(5, dtype='i4')

This comment has been minimized.

@kwgoodman

kwgoodman Feb 7, 2012

Owner

Maybe use a smaller example like la.ones(3)

@kwgoodman

kwgoodman Feb 7, 2012

Owner

Maybe use a smaller example like la.ones(3)

la/flarry.py
+ else:
+ if shape is None:
+ raise ValueError("Either `label` or `shape` must be supplied.")
+ label = [range(i) for i in shape]

This comment has been minimized.

@kwgoodman

kwgoodman Feb 7, 2012

Owner

I don't think this line is needed since the default is for larry to use range(n) labels. Same goes for la.ones

@kwgoodman

kwgoodman Feb 7, 2012

Owner

I don't think this line is needed since the default is for larry to use range(n) labels. Same goes for la.ones

la/flarry.py
@@ -649,6 +650,279 @@ def intersection(axis, *args):
rc = list(rc)
rc.sort()
return rc
+# Instantiation shortcuts---------------------------------------------------
+
+def arange(*args, **kwargs):

This comment has been minimized.

@kwgoodman

kwgoodman Feb 7, 2012

Owner

It does something too different from np.arange. Is there another name we could use for this function?

@kwgoodman

kwgoodman Feb 7, 2012

Owner

It does something too different from np.arange. Is there another name we could use for this function?

RELEASE.rst
@@ -16,6 +16,7 @@ la 0.6
- la.isaligned() returns True if two larrys are aligned along specified axis
- la.sortby() sorts a larry by a row or column specified by its label
- la.align_axis() aligns multiple larrys along (possibly) different axes
+- la.arange(), la.ones(), and la.zeros() provide quick ways to get test data

This comment has been minimized.

@kwgoodman

kwgoodman Feb 7, 2012

Owner

Remove: "provide quick ways to get test data"

But that's what I'll use it for ;)

@kwgoodman

kwgoodman Feb 7, 2012

Owner

Remove: "provide quick ways to get test data"

But that's what I'll use it for ;)

@kwgoodman

This comment has been minimized.

Show comment
Hide comment
@kwgoodman

kwgoodman Feb 7, 2012

Owner

These functions will come in handy!

Can you add the .. autofunction:: la.ones etc to the sphinx doc?

Owner

kwgoodman commented Feb 7, 2012

These functions will come in handy!

Can you add the .. autofunction:: la.ones etc to the sphinx doc?

@kwgoodman

This comment has been minimized.

Show comment
Hide comment
@kwgoodman

kwgoodman Feb 7, 2012

Owner

I noticed that in numpy np.ones and np.zeros just call np.empty with fill values of 1 and 0, respectively.

Maybe we should go that way too? So if we made a la.empty then la.ones and la.zeros could use that and just fill the arrays. That would be cleaner (less duplicate code) plus it would give us a la.empty function.

Owner

kwgoodman commented Feb 7, 2012

I noticed that in numpy np.ones and np.zeros just call np.empty with fill values of 1 and 0, respectively.

Maybe we should go that way too? So if we made a la.empty then la.ones and la.zeros could use that and just fill the arrays. That would be cleaner (less duplicate code) plus it would give us a la.empty function.

@stroxler

This comment has been minimized.

Show comment
Hide comment
@stroxler

stroxler Feb 7, 2012

I can do that. Empty() can't be tested easily, but that is no problem. I guess you could view the other tests as testing it.

stroxler commented Feb 7, 2012

I can do that. Empty() can't be tested easily, but that is no problem. I guess you could view the other tests as testing it.

la/flarry.py
+ array([0.0, 1.0, 2.0, 3.0, 4.0], dtype=np.float32)
+
+ A multi-dimensional arange:
+ >>> la.arange(2,3,3)

This comment has been minimized.

@kwgoodman

kwgoodman Feb 7, 2012

Owner

lrange is a good name. Change arange to lrange in several places.

@kwgoodman

kwgoodman Feb 7, 2012

Owner

lrange is a good name. Change arange to lrange in several places.

la/flarry.py
+
+# Instantiation shortcuts---------------------------------------------------
+
+def lrange(*args, **kwargs):

This comment has been minimized.

@kwgoodman

kwgoodman Feb 7, 2012

Owner

Is there a way to get rid of _args, *_kwargs? Would this work:

lrange(shape=None, label=None, start=0, step=1, dtype=None)

?

@kwgoodman

kwgoodman Feb 7, 2012

Owner

Is there a way to get rid of _args, *_kwargs? Would this work:

lrange(shape=None, label=None, start=0, step=1, dtype=None)

?

la/flarry.py
+ # make the data, adjust, and put together
+ total = np.product(np.array(shape))
+ data = np.arange(total, **kwargs).reshape(shape)
+ data *= step; data += start

This comment has been minimized.

@kwgoodman

kwgoodman Feb 7, 2012

Owner

Can step and start be passed directly to np.arange?

@kwgoodman

kwgoodman Feb 7, 2012

Owner

Can step and start be passed directly to np.arange?

This comment has been minimized.

@stroxler

stroxler Feb 7, 2012

Not as is, but I will adjust them so they can. It will lead to a small efficiency gain, at the price of very slightly worse readability.

@stroxler

stroxler Feb 7, 2012

Not as is, but I will adjust them so they can. It will lead to a small efficiency gain, at the price of very slightly worse readability.

This comment has been minimized.

@kwgoodman

kwgoodman Feb 7, 2012

Owner

I think (assuming I coded it right)

data = np.arange(start=start, stop=step*total, step=step, dtype=dtype).reshape(shape)

is more readable than

data = np.arange(total, **kwargs).reshape(shape)
data *= step
data += start

And it is best not to re-implement what is already in numpy.

@kwgoodman

kwgoodman Feb 7, 2012

Owner

I think (assuming I coded it right)

data = np.arange(start=start, stop=step*total, step=step, dtype=dtype).reshape(shape)

is more readable than

data = np.arange(total, **kwargs).reshape(shape)
data *= step
data += start

And it is best not to re-implement what is already in numpy.

la/flarry.py
+
+ Using the 'label' argument:
+
+ >>> la.arange(label=[['a', 'b']])

This comment has been minimized.

@kwgoodman

kwgoodman Feb 7, 2012

Owner

la.arange should be la.empty

@kwgoodman

kwgoodman Feb 7, 2012

Owner

la.arange should be la.empty

la/flarry.py
+ x
+ array([0, 0, 0, -7, 987], dtype=np.int32)
+
+ A multi-dimensional larry:

This comment has been minimized.

@kwgoodman

kwgoodman Feb 7, 2012

Owner

Add blank line

@kwgoodman

kwgoodman Feb 7, 2012

Owner

Add blank line

la/flarry.py
+ label = [range(i) for i in shape]
+ data = np.empty(shape, dtype, order)
+ return larry(data, label)
+

This comment has been minimized.

@kwgoodman

kwgoodman Feb 7, 2012

Owner

Two extra blank lines crept in.

@kwgoodman

kwgoodman Feb 7, 2012

Owner

Two extra blank lines crept in.

la/flarry.py
+ order : {'C', 'F'}, optional
+ Whether to store multidimensional data in C- or Fortran-contiguous
+ (row- or column-wise) order in memory.
+

This comment has been minimized.

@kwgoodman

kwgoodman Feb 7, 2012

Owner

Remove extra blank line

@kwgoodman

kwgoodman Feb 7, 2012

Owner

Remove extra blank line

@kwgoodman

This comment has been minimized.

Show comment
Hide comment
@kwgoodman

kwgoodman Feb 7, 2012

Owner

Could you add See Also sections? ones, zeros, empty, lrange could all be in each others docstrings.

Owner

kwgoodman commented Feb 7, 2012

Could you add See Also sections? ones, zeros, empty, lrange could all be in each others docstrings.

@stroxler

This comment has been minimized.

Show comment
Hide comment
@stroxler

stroxler Feb 7, 2012

The _args and *_kwargs cannot be dropped if we want the function to behave the way we had initially decided. But we could make it follow the same rules that the others follow, in which case they can be.

The issue is that without _args, you can't get something like la.lrange(5,4,3) to work. It would have to be la.lrange((5,4,3)). That's why all these functions had *args and *_kwargs in the first place. But I don't see a strong reason to prefer the current call. It is more convenient, but if the docstring is confusing does that matter?

stroxler commented Feb 7, 2012

The _args and *_kwargs cannot be dropped if we want the function to behave the way we had initially decided. But we could make it follow the same rules that the others follow, in which case they can be.

The issue is that without _args, you can't get something like la.lrange(5,4,3) to work. It would have to be la.lrange((5,4,3)). That's why all these functions had *args and *_kwargs in the first place. But I don't see a strong reason to prefer the current call. It is more convenient, but if the docstring is confusing does that matter?

@kwgoodman

This comment has been minimized.

Show comment
Hide comment
@kwgoodman

kwgoodman Feb 7, 2012

Owner

Good point. Let's go with the shape input so that lrange matches la.ones, zeros and empty instead la.rand, randn.

Owner

kwgoodman commented Feb 7, 2012

Good point. Let's go with the shape input so that lrange matches la.ones, zeros and empty instead la.rand, randn.

@kwgoodman

This comment has been minimized.

Show comment
Hide comment
@kwgoodman

kwgoodman Feb 7, 2012

Is this a typo? Should be possible to do la.lrange((2,3)) without supplying a label.

Is this a typo? Should be possible to do la.lrange((2,3)) without supplying a label.

@kwgoodman

This comment has been minimized.

Show comment
Hide comment
@kwgoodman

kwgoodman Feb 7, 2012

I keep marking this line as not needed. Is there a reason to keep it?

I keep marking this line as not needed. Is there a reason to keep it?

@stroxler

This comment has been minimized.

Show comment
Hide comment
@stroxler

stroxler Feb 7, 2012

That was a typo. Should be fixed now.

stroxler commented Feb 7, 2012

That was a typo. Should be fixed now.

@kwgoodman kwgoodman merged commit c94ffef into kwgoodman:master Feb 7, 2012

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