Skip to content

convert modelchain.ipynb to rst, remove nbsphinx #568

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

Merged
merged 9 commits into from
Sep 9, 2018

Conversation

wholmgren
Copy link
Member

@wholmgren wholmgren commented Sep 8, 2018

  • Closes readthedocs are failing #567
  • I am familiar with the contributing guidelines.
  • Fully tested. Added and/or modified tests to ensure correct behavior for all reasonable inputs. Tests (usually) must pass on the TravisCI and Appveyor testing services.
  • Updates entries to docs/sphinx/source/api.rst for API changes.
  • Adds description and name entries in the appropriate docs/sphinx/source/whatsnew file for all changes.
  • Code quality and style is sufficient. Passes LGTM and SticklerCI checks.
  • New code is fully documented. Includes sphinx/numpydoc compliant docstrings and comments in the code where necessary.
  • Pull request is nearly complete and ready for detailed review.

New documentation using the new modelchain.rst file:

https://wholmgren-pvlib-python-new.readthedocs.io/en/mcdocclean/modelchain.html

Last successful build on pvlib/master using the old modelchain.ipynb file:

https://pvlib-python.readthedocs.io/en/latest/modelchain.html

@wholmgren wholmgren added this to the 0.6.0 milestone Sep 8, 2018

ModelChain(poorly_specified_system, location)

If our goal is simply to get the object constructed, we can specify the
Copy link
Member Author

Choose a reason for hiding this comment

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

This section will need to change due to #555

of the modeling process while providing user-control and remaining
extensible. This guide aims to build users' understanding of the
ModelChain class. It assumes some familiarity with object-oriented
ipython in Python, but most information should be understandable even
Copy link
Member Author

Choose a reason for hiding this comment

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

typo

mc.run_model(times=weather.index, weather=weather)

The ModelChain attempted to execute the PVSystem object's
:py:meth:`~pvlib.pvsystem.PVSystem.singlediode` method, and the method
Copy link
Member Author

Choose a reason for hiding this comment

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

if reworking this section, replace singlediode with a different method

Copy link
Member

Choose a reason for hiding this comment

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

cec should be our default example

Copy link
Member Author

Choose a reason for hiding this comment

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

I decided to remove this section that referenced singlediode because #555 disallowed this "feature".

The remaining examples demonstrate sapm and pvwatts. I'm ok adding another example that demonstrates the behavior when pulling from the cec database if you don't think it would be pedantic. Not sure about pvsyst/desoto.

Copy link
Member

Choose a reason for hiding this comment

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

OK with me to remove it. The scope of this PR is to rehost the example, so we can push off expanding the example to a follow on.

---------------------------------

The ModelChain class has a lot going in inside it in order to make
users' ipython as simple as possible.
Copy link
Member Author

Choose a reason for hiding this comment

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

another code --> ipython replace all issue

The key parts of ModelChain are:

1. The :py:meth:`ModelChain.run_model() <.ModelChain.run_model>` method
1. A set of methods that wrap and call the PVSystem methods.
Copy link
Member Author

Choose a reason for hiding this comment

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

2

Copy link
Member Author

Choose a reason for hiding this comment

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

and indent

Wrapping methods into a unified API
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

Readers may notice that the source ipython of the ModelChain.run_model
Copy link
Member Author

Choose a reason for hiding this comment

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

replace all again

The ModelChain.pvwatts_dc method calls the pvwatts_dc method of the
PVSystem object that we supplied using data that is stored in its own
``effective_irradiance`` and ``temps`` attributes. Then it assigns the
result to the ``dc`` attribute of the ModelChain object. The ipython below
Copy link
Member Author

Choose a reason for hiding this comment

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

and again

@mikofski
Copy link
Member

mikofski commented Sep 8, 2018

Does this also drop the jupyter requirement too? If so it will be a huge relief for those of us running headless, no gui, eg as in Windows Subsystem for Linux. Two less requirements to install that come with a lot of overhead, so thanks! 😄

@@ -829,8 +829,7 @@ def bird(zenith, airmass_relative, aod380, aod500, precipitable_water,

`SERI/TR-642-761 <http://rredc.nrel.gov/solar/pubs/pdfs/tr-642-761.pdf>`_

`Error Reports
<http://rredc.nrel.gov/solar/models/clearsky/error_reports.html>`_
`Error Reports <http://rredc.nrel.gov/solar/models/clearsky/error_reports.html>`_
Copy link

Choose a reason for hiding this comment

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

E501 line too long (85 > 79 characters)

Copy link
Member Author

Choose a reason for hiding this comment

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

This was causing doc build errors when it was two lines. Not sure why or how to fix it. I'm ok with allowing it to be too long.

@@ -934,8 +934,7 @@ def equation_of_time_pvcdrom(dayofyear):
`PVCDROM`_ is a website by Solar Power Lab at Arizona State
University (ASU)

.. _PVCDROM:
http://www.pveducation.org/pvcdrom/2-properties-sunlight/solar-time
.. _PVCDROM: http://www.pveducation.org/pvcdrom/2-properties-sunlight/solar-time
Copy link

Choose a reason for hiding this comment

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

E501 line too long (84 > 79 characters)

Copy link
Member Author

Choose a reason for hiding this comment

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

same as above

@wholmgren
Copy link
Member Author

@mikofski I believe we can drop jupyter. The docs are now mostly building without jupyter present. For some reason the ipython directive no longer executes the code in the pvsystem.rst and timetimezones.rst files. I think it's unrelated but not 100% sure.

I am a bit confused as to why you're concerned about a jupyter requirement for the doc builds but also in favor of adding scipy to the base requirements. Feels like I'm missing something.

@mikofski
Copy link
Member

mikofski commented Sep 9, 2018

Well scipy can be used for many things other than pvlib, whereas Jupyter is mainly used as a graphical user interface, which is pointless in a headless environment. But it doesn't really matter, it's more just the principle I guess.

@@ -829,8 +829,7 @@ def bird(zenith, airmass_relative, aod380, aod500, precipitable_water,

`SERI/TR-642-761 <http://rredc.nrel.gov/solar/pubs/pdfs/tr-642-761.pdf>`_

`Error Reports
<http://rredc.nrel.gov/solar/models/clearsky/error_reports.html>`_
`Error Reports <http://rredc.nrel.gov/solar/models/clearsky/error_reports.html>`_
Copy link

Choose a reason for hiding this comment

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

E501 line too long (85 > 79 characters)

@@ -934,8 +934,7 @@ def equation_of_time_pvcdrom(dayofyear):
`PVCDROM`_ is a website by Solar Power Lab at Arizona State
University (ASU)

.. _PVCDROM:
http://www.pveducation.org/pvcdrom/2-properties-sunlight/solar-time
.. _PVCDROM: http://www.pveducation.org/pvcdrom/2-properties-sunlight/solar-time
Copy link

Choose a reason for hiding this comment

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

E501 line too long (84 > 79 characters)

@stickler-ci
Copy link

stickler-ci bot commented Sep 9, 2018

Your configuration file uses an unknown linter. The flake8 linter is not a supported linter.

@stickler-ci
Copy link

stickler-ci bot commented Sep 9, 2018

Your configuration file uses an unknown linter. The flake8 linter is not a supported linter.

@wholmgren
Copy link
Member Author

I cannot reliably reproduce the ipython execution failures locally or on rtd. I'll merge and hope for the best. At least the docs should build again.

@wholmgren wholmgren merged commit 6c6685a into pvlib:master Sep 9, 2018
@wholmgren
Copy link
Member Author

Aaaaand the docs build but fail to execute the ipython directive blocks in pvsystem.rst and timetimezones.rst. :(

@@ -829,8 +829,7 @@ def bird(zenith, airmass_relative, aod380, aod500, precipitable_water,

`SERI/TR-642-761 <http://rredc.nrel.gov/solar/pubs/pdfs/tr-642-761.pdf>`_

`Error Reports
<http://rredc.nrel.gov/solar/models/clearsky/error_reports.html>`_
`Error Reports <http://rredc.nrel.gov/solar/models/clearsky/error_reports.html>`_
Copy link

Choose a reason for hiding this comment

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

E501 line too long (85 > 79 characters)

@@ -934,8 +934,7 @@ def equation_of_time_pvcdrom(dayofyear):
`PVCDROM`_ is a website by Solar Power Lab at Arizona State
University (ASU)

.. _PVCDROM:
http://www.pveducation.org/pvcdrom/2-properties-sunlight/solar-time
.. _PVCDROM: http://www.pveducation.org/pvcdrom/2-properties-sunlight/solar-time
Copy link

Choose a reason for hiding this comment

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

E501 line too long (84 > 79 characters)

@wholmgren wholmgren mentioned this pull request Dec 10, 2018
8 tasks
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