Skip to content

TST, MAINT: Lots of new tests for fromnumeric.py - #7027

Merged
charris merged 1 commit into
numpy:masterfrom
gfyoung:test_fromnumeric_expand
Jan 19, 2016
Merged

charris merged 1 commit into
numpy:masterfrom
gfyoung:test_fromnumeric_expand

Conversation

@gfyoung

@gfyoung gfyoung commented Jan 16, 2016

Copy link
Copy Markdown
Contributor

Follow-up from #7000 in which I found there seemed to be a gap in testing for the numpy/numpy/core/fromnumeric.py file. This PR improves the coverage on the methods defined in that file.

@gfyoung gfyoung changed the title TST: Lots of new tests for fromnumeric.py TST, MAINT: Lots of new tests for fromnumeric.py Jan 16, 2016
@charris

charris commented Jan 17, 2016

Copy link
Copy Markdown
Member

Please don't rebase.

@gfyoung

gfyoung commented Jan 18, 2016

Copy link
Copy Markdown
Contributor Author

PR is no longer WIP. Travis + Appveyor are both passing. Should be good to merge now.

Comment thread numpy/core/tests/test_multiarray.py Outdated

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.

Should keep the first version in order to test the funtion, not the method.

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.

Same below.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Done.

@charris

charris commented Jan 18, 2016

Copy link
Copy Markdown
Member

Looks generally good, but the common mistake is to substitute methods for functions. The two aren't the same despite the apparent redundancy. The methods should actually be tested in the numpy/core/tests/test_multiarray.py huge TestMethods class. Note that Numeric was the first version of what became Numpy and didn't have many methods.

@gfyoung

gfyoung commented Jan 19, 2016

Copy link
Copy Markdown
Contributor Author

@charris : Changes made and Travis / Appveyor are happy.

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.

Why remove this file? fromnumeric.py is a package containing a number of functions. put is also a function implemented in fromnumeric.py.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I added that file myself from #7000 that served as a stopgap measure to test that put would not accept non-ndarray args. Also, a lot of the tests of fromnumeric.py functions are located elsewhere in the tests.

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.

Hmm, at some point we need to reorganize numpy/core, but I suppose that is for another day.

@charris

charris commented Jan 19, 2016

Copy link
Copy Markdown
Member

I think you need to check carefully where functions/methods etc are defined. The easy way to check functions in ipython is to use the ? help option on the function name. Or you can look at the __all__ list in the modules to see what functions are exported. You can check ndarray methods with ndarray.<themethod>?.

@gfyoung

gfyoung commented Jan 19, 2016

Copy link
Copy Markdown
Contributor Author

If you look at the functions defined in fromnumeric.py, almost all of them are tested elsewhere in the code-base, even though they are in the __all__ array. For example np.prod is defined here, but the test is here.

My original intention was to write an entire test_fromnumeric.py but after searching the code base for many of these functions, I realized that I would just be duplicating already existing tests.

charris added a commit that referenced this pull request Jan 19, 2016
TST, MAINT: Lots of new tests for fromnumeric.py
@charris
charris merged commit 275fa79 into numpy:master Jan 19, 2016
@charris

charris commented Jan 19, 2016

Copy link
Copy Markdown
Member

Thanks @gfyoung .

@gfyoung
gfyoung deleted the test_fromnumeric_expand branch January 19, 2016 20:59
@homu homu mentioned this pull request Jan 19, 2016
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.

2 participants