Skip to content

Conversation

@wholmgren
Copy link
Member

As discussed in #13.

I briefly looked into adding a real test for this, but it wasn't immediately obvious how to do it. Instead, I ran the test suite on a virtualenv without scipy. Here's the output, as expected:

$ nosetests pvlib/test/test_clearsky.py -v
pvlib.test.test_clearsky.test_ineichen_required ... ERROR
pvlib.test.test_clearsky.test_ineichen_supply_linke ... ok
pvlib.test.test_clearsky.test_ineichen_solpos ... ok
pvlib.test.test_clearsky.test_ineichen_airmass ... ok
pvlib.test.test_clearsky.test_ineichen_keys ... ok
pvlib.test.test_clearsky.test_haurwitz ... ok
pvlib.test.test_clearsky.test_haurwitz_keys ... ok
pvlib.test.test_clearsky.test_disc_keys ... ok
pvlib.test.test_clearsky.test_disc_value ... ok

======================================================================
ERROR: pvlib.test.test_clearsky.test_ineichen_required
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/Users/holmgren/.pyenv/versions/pvlib-3.4.3/lib/python3.4/site-packages/nose/case.py", line 198, in runTest
    self.test(*self.arg)
  File "/Users/holmgren/git_repos/pvlib-python/pvlib/test/test_clearsky.py", line 34, in test_ineichen_required
    clearsky.ineichen(times, tus)
  File "/Users/holmgren/git_repos/pvlib-python/pvlib/clearsky.py", line 134, in ineichen
    'supply your own turbidities.')
nose.proxy.ImportError: The Linke turbidity lookup table requires scipy. You can still use clearsky.ineichen if you supply your own turbidities.
-------------------- >> begin captured logging << --------------------
pvlib: DEBUG: creating Location object
pvlib: DEBUG: using PyEphem to calculate solar position
pvlib: DEBUG: tz_localize to US/Arizona and then tz_convert to UTC
pvlib: DEBUG: irradiance.extraradiation()
pvlib: DEBUG: Calculating ET rad using Spencer method
pvlib: DEBUG: using PyEphem to calculate solar position
pvlib: DEBUG: tz_localize to US/Arizona and then tz_convert to UTC
--------------------- >> end captured logging << ---------------------

----------------------------------------------------------------------
Ran 9 tests in 1.986s

FAILED (errors=1)

@wholmgren wholmgren added this to the 0.1 milestone Mar 13, 2015
@wholmgren
Copy link
Member Author

Resolved the merge conflict in the whats new file.

@wholmgren
Copy link
Member Author

@bmu or @Calama-Consulting are we good to go on this one?

bmu added a commit that referenced this pull request Mar 19, 2015
@bmu bmu merged commit 4a36bb2 into pvlib:master Mar 19, 2015
@bmu
Copy link
Contributor

bmu commented Mar 19, 2015

Ooops, maybe I was to hasty. When I run the tests without scipy I get for errors:

  • pvlib.test.test_clearsky.test_ineichen_required ... ERROR
  • pvlib.test.test_clearsky.test_disc_value ... ok
    Failure: ImportError (The Linke turbidity lookup table requires scipy. You can still use clearsky.ineichen if you supply your own turbidities.) ... ERROR
  • pvlib.test.test_location.test_location_print_pytz ... ok
    Failure: ImportError (The Linke turbidity lookup table requires scipy. You can still use clearsky.ineichen if you supply your own turbidities.) ... ERROR
  • pvlib.test.test_solarposition.test_calc_time ... ERROR

Not sure, where the errors are thrown in all cases (I don't see any use of ineichen or scipy in Location), I just copied the output of nosetests -v -w pvlib --with-coverage --cover-package=pvlib.

I think we should skip all tests that use scipy, if scipy is not installed.
Should I revert the merge?

@wholmgren
Copy link
Member Author

Ok, that was very confusing but I tracked it down to test_irradiance.py and test_pvsystem.py failing on module import without scipy. They both use clearsky.ineichen to calculate the irradiance data used in later tests. I changed the calls to use a fixed turbidity.

The nosetests log now looks right to me.

I agree that we should skip the tests that require scipy if scipy is not installed, however, I would suggest that we make that a 0.2 milestone.

@wholmgren
Copy link
Member Author

Ah, not sure what the best git approach is here, but I ended up making a new PR in #34 to resolve this.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants