Skip to content

Class based testing #12

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
bmu opened this issue Mar 1, 2015 · 13 comments
Closed

Class based testing #12

bmu opened this issue Mar 1, 2015 · 13 comments
Assignees
Labels
Milestone

Comments

@bmu
Copy link
Contributor

bmu commented Mar 1, 2015

I would like to implement real nose test cases or pandas based TestCases

Also pandas like testscripts for lokal testing would be nice (see https://github.com/pydata/pandas).

At the moment I don't know how to run the test suite locally.

@wholmgren
Copy link
Member

nosetests will try to run any python modules labeled *test* that are in the current directory or any subdirectories. You can also give it an explicit test module e.g. nosetests pvlib/tests/test_tmy.py. That being said, I don't have any objection to adding a test.sh similar to the one in pandas.

There are certainly a lot of tests that could be made cleaner with class based testing and setUp methods.

@bmu
Copy link
Contributor Author

bmu commented Mar 2, 2015

Yes, it should, however if I run it in my local repository from the base folder (where setup.py is located) the output of nosetests is

----------------------------------------------------------------------
Ran 0 tests in 0.000s

OK

Not sure whats going on here, as the coverage seems to work:

nosetests -v -w pvlib --with-coverage --cover-package=pvlib

Name                  Stmts   Miss  Cover   Missing
---------------------------------------------------
pvlib                    11      0   100%   
pvlib.atmosphere         38     29    24%   56-57, 97-99, 144-146, 215-242
pvlib.clearsky          106     91    14%   106-216, 265-273, 280-283, 350-388
pvlib.irradiance        196    169    14%   85-115, 132, 149, 168, 195-202, 230-238, 267-279, 287-290, 333-363, 428-444, 502-516, 582-586, 677-695, 780-799, 902-926, 983-988, 1115-1186, 1245-1354
pvlib.location           22     15    32%   50-66, 71-73
pvlib.pvsystem          201    177    12%   10-11, 89-105, 161-166, 239-254, 438-458, 521-548, 553-561, 627-660, 725-748, 847-896, 938-969, 978-979, 987-988, 999-1010, 1084-1101
pvlib.solarposition     174    159     9%   41-52, 86-118, 122-133, 160-204, 268-380, 422-440, 456-466
pvlib.spa_c_files         0      0   100%   
pvlib.tmy               106     88    17%   159-182, 187-190, 197-201, 207-213, 218-233, 392-404, 426-438, 443-498
pvlib.tools              46     30    35%   35-36, 57-58, 79-80, 102-103, 120-137, 155-163, 183-186
pvlib.version             1      0   100%   
---------------------------------------------------
TOTAL                   901    758    16%   
----------------------------------------------------------------------
Ran 0 tests in 0.001s

OK

@wholmgren
Copy link
Member

Strange. I ran that exact command from the same directory and it worked as expected.

$ nosetests --version
nosetests version 1.3.4

$ coverage --version
Coverage.py, version 3.7.1.  http://nedbatchelder.com/code/coverage

@robwandrews
Copy link
Contributor

In some cases you need to enable the --exe option for it to work . That being said the following worked for me:

nosetests -v 

in the pvlib-python directory

@bmu
Copy link
Contributor Author

bmu commented Mar 4, 2015

I use the same versions. However I'm on linux 64bit, maybe something goes wrong here. Still couldn't figure out, what the problem is, but will have time on Friday to check this.

@dacoex
Copy link
Contributor

dacoex commented Mar 4, 2015

@bmu
take a look at this:
https://launchpad.net/~pythonxy/+archive/ubuntu/pythonxy-devel
may ease up your updating with auto built daily packages.

@bmu
Copy link
Contributor Author

bmu commented Mar 8, 2015

I solved my problem (The reason was, that I was trying to test pvlib on a NTFS drive on my new machine, which was mounted so that the files were executable and nosetests skippes files, that are executable!).

@wholmgren: Do you think it makes sence, that I open a pull request that changes the unit tests to "real" unittests test now, or is this something for a later release? I could also create the milestones (see #11) first.

@wholmgren
Copy link
Member

There are definitely problems with the existing test modules that classes traditionally fix:

  • The set up code is run once at the beginning of the module and reused for all tests
  • The set up code is executed when the module is imported since it's not protected by a class or if __name__ == '__main__' statement
  • I used copy/paste/modify to write many functions

I'll take most (all?) of the blame for the fast and easy but unsustainable pattern. Some of the tests, particularly those that use solar position, will probably need to be made faster if the set up is executed multiple times.

Do you want to remove the nose dependency for development or just clean up the copy/paste mess?

I'm reluctant to support a rule along the lines of "all tests must be in classes that inherit from pandas TestCase or unittest.TestCase". I would prefer to use test classes where they will clearly improve the code, and leave functions where they will not. I also think that less experienced python developers will have an easier time making tests if they only need to be functions that are named "test_*".

In any case, I suggest that any test suite overhaul be done after the 0.1 release.

@bmu
Copy link
Contributor Author

bmu commented Mar 9, 2015

My intention to move to Pandas TestCases is not to have "nicer code" or that we use classes without a need to.
The reason is, that these test cases really test a series or a DataFrame: they test for expected values and for the expected index.
This is especially useful because often time series are used as input for pvlib functions. My experience from everyday work with functions (not classes;-) that use Pandas and numpy is, that the preservation of Datetime indexes is sometimes a difficulty and can lead to unexpected results, that are not easy to detect.

Also for unexperienced programmers it may be usefull to have something like a template for testing, which could be provided by an existing test.

No Problem with skipping this for now, but I think these Tests are really useful for future releases.

@wholmgren
Copy link
Member

That all makes sense. Good point regarding the templates too.
On Sun, Mar 8, 2015 at 11:08 PM bmu [email protected] wrote:

My intention to move to Pandas TestCases is not to have "nicer code" or
that we use classes without a need to.
The reason is, that these test cases really test a series or a DataFrame:
they test for expected values and for the expected index.
This is especially useful because often time series are used as input for
pvlib functions. My experience from everyday work with functions (not
classes;-) that use Pandas and numpy is, that the preservation of Datetime
indexes is sometimes a difficulty and can lead to unexpected results, that
are not easy to detect.

Also for unexperienced programmers it may be usefull to have something
like a template for testing, which could be provided by an existing test.

No Problem with skipping this for now, but I think these Tests are really
useful for future releases.


Reply to this email directly or view it on GitHub
#12 (comment).

@bmu bmu added the testing label Mar 12, 2015
@bmu bmu added this to the 0.2 milestone Mar 12, 2015
@bmu bmu self-assigned this Mar 13, 2015
@bmu bmu modified the milestones: Someday, 0.2 Jun 21, 2015
@wholmgren
Copy link
Member

A couple of things related to the stalled conversation above...

The nose documentation is now warning people that it's no longer in active development and recommends that new projects use pytest or just unittest. I've not used pytest, but it looks nice.

I added a bunch of assert_series_equal and assert_frame_equal statements to the test code, so I think that this covers most of @bmu's concerns in his last comment and I wonder if this issue can be closed.

@bmu
Copy link
Contributor Author

bmu commented Feb 2, 2016

So feel free to close it!

Am 2. Februar 2016 01:06:26 MEZ, schrieb Will Holmgren [email protected]:

A couple of things related to the stalled conversation above...

The nose documentation is now warning people that it's no longer in
active development and recommends that new projects use pytest or just
unittest. I've not used pytest, but it looks nice.

I added a bunch of assert_series_equal and assert_frame_equal
statements to the test code, so I think that this covers most of @bmu's
concerns in his last
comment
and I wonder if this issue can be closed.


Reply to this email directly or view it on GitHub:
#12 (comment)

Diese Nachricht wurde von meinem Android-Mobiltelefon mit K-9 Mail gesendet.

@wholmgren
Copy link
Member

thanks @bmu, closing.

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

No branches or pull requests

4 participants