-
Notifications
You must be signed in to change notification settings - Fork 1.1k
Update intro tutorial (#1116) #1144
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
Conversation
Make the procedural and OO results match exactly and highlight the use of `iotools` to download meteorological data.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @Peque this is great. One comment below and can you also add a note to the 0.9 whats new file?
docs/sphinx/source/introtutorial.rst
Outdated
includes irradiation, temperature and wind speed. Note that PVGIS uses | ||
different naming conventions, so it is required to rename the weather data | ||
variables before using them. PVGIS weather data is already UTC-localized, so | ||
conversion to local timezone is optional. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The text itself is pretty neutral, but I don't want to imply that we favor PVGIS. I'm wondering if we need a sentence here like See :ref:`iotools` for other weather data sources.
or maybe even something more explicit.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pushed a fixup commit with a little change. Feel free to rephrase it as you'd like if you still prefer some other way to say it. 😊
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @Peque .
My only comment is outside the scope of this tutorial: it is unfortunate that the key for getting 'sandia'
inverter parameters using retrieve_sam
is 'cecinverter'
. "sandia", "SAM" and "cec" all appear in this line of code; no surprise that novices get confused.
By the way, just in case you missed my comment above, I could add some transformations to the downloaded TMY data to avoid possible confusion to new users, in particular:
Like this: --- a/docs/sphinx/source/introtutorial.rst
+++ b/docs/sphinx/source/introtutorial.rst
@@ -71,7 +71,11 @@ includes irradiation, temperature and wind speed.
latitude, longitude, name, altitude, timezone = location
weather = pvlib.iotools.get_pvgis_tmy(latitude, longitude)[0]
weather = weather.rename(columns=variables_translation)
+ weather.index = weather.index.map(lambda t: t.replace(year=2019))
weather.index.name = "utc_time"
+ # Optional conversion to local timezone
+ weather.index = weather.index.tz_convert(timezone)
+ weather.index.name = "local_time"
tmys.append(weather) Or would you rather leave it as UTC and as isolated months from different years? |
let's keep it as is - the "intro example" can't do everything. thanks @Peque! |
Excellent, thanks for the work! It might be a good time to correct the inaccurate coordinates in If you're interested: import pandas as pd
import matplotlib.pyplot as plt
- # very approximate
# latitude, longitude, name, altitude, timezone
coordinates = [
- (30, -110, 'Tucson', 700, 'Etc/GMT+7'),
- (35, -105, 'Albuquerque', 1500, 'Etc/GMT+7'),
- (40, -120, 'San Francisco', 10, 'Etc/GMT+8'),
- (50, 10, 'Berlin', 34, 'Etc/GMT-1'),
+ (32.2, -111.0, 'Tucson', 700, 'Etc/GMT+7'),
+ (35.1, -106.6, 'Albuquerque', 1500, 'Etc/GMT+7'),
+ (37.8, -122.4, 'San Francisco', 10, 'Etc/GMT+8'),
+ (52.5, 13.4, 'Berlin', 34, 'Etc/GMT-1'),
] Here's the commit: |
ModelChain.run_model
documentation #1116Tests added(N/A)Updates entries to(N/A)docs/sphinx/source/api.rst
for API changes.Adds description and name entries in the appropriate "what's new" file in(N/A)docs/sphinx/source/whatsnew
for all changes. Includes link to the GitHub Issue with:issue:`num`
or this Pull Request with:pull:`num`
. Includes contributor name and/or GitHub username (link with:ghuser:`user`
).New code is fully documented. Includes numpydoc compliant docstrings, examples, and comments where necessary.(N/A)This MR updates the example code in the intro tutorial. A couple of things to note:
iotools
functionAlso, I could add some transformations to the downloaded TMY data to avoid possible confusion to new users, in particular: