Skip to content

Conversation

mikofski
Copy link
Member

  • change units for pressure to mbars

* change units for pressure to mbars
@adriesse
Copy link
Member

Does this have any repercussions for the solar position calculations? It seems that Pa are used everywhere else in PVLIB. Is this converted to mbar where necessary? pvlib.solarposition.get_solarposition appears to use Pa.

@mikofski
Copy link
Member Author

No, the conversion is already handled in pvlib.solarposition.spa_python by dividing pressure by 100(mbar/PA) https://github.com/pvlib/pvlib-python/blob/master/pvlib/solarposition.py

@wholmgren
Copy link
Member

Good catch, Mark. I suppose we could also change the API to make it more consistent, but I prefer the quick doc fix. Can you add a note to the whatsnew?

@wholmgren wholmgren added this to the 0.4.5 milestone May 26, 2017
@mikofski
Copy link
Member Author

mikofski commented Jun 2, 2017

Hi @wholmgren , I added a somewhat verbose note in "whats new for v0.4.5" b/c I wanted it to be absolutely clear that this docstring change re: units of pressure in the low-level spa.py module should have no effect on almost all users.

Update docstring in pvlib.spa.solar_position - change units of pressure to millibars. NOTE: units of pressure in pvlib.solar_position.spa_python and pvlib.solar_position.spa_c are still Pascals. This update should have no effect on most users, since it only applies to the low-level spa.py module.

@wholmgren
Copy link
Member

thanks mark

@wholmgren wholmgren merged commit 69d1bf3 into pvlib:master Jun 2, 2017
@mikofski mikofski deleted the spa_solarposition_mbars branch August 8, 2017 21:28
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.

3 participants