Skip to content

Change sapm to expect effective_irradiance in W/m2#609

Closed
cwhanse wants to merge 11 commits into
pvlib:masterfrom
cwhanse:eff_irrad_units
Closed

Change sapm to expect effective_irradiance in W/m2#609
cwhanse wants to merge 11 commits into
pvlib:masterfrom
cwhanse:eff_irrad_units

Conversation

@cwhanse

@cwhanse cwhanse commented Oct 19, 2018

Copy link
Copy Markdown
Member

pvlib python pull request guidelines

Thank you for your contribution to pvlib python! You may delete all of these instructions except for the list below.

You may submit a pull request with your code at any stage of completion.

The following items must be addressed before the code can be merged. Please don't hesitate to ask for help if you're unsure of how to accomplish any of the items below:

  • Closes Effective irradiance units #472
  • 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.

Brief description of the problem and proposed solution (if not already fully described in the issue linked to above):

Comment thread pvlib/pvsystem.py Outdated
@wholmgren

Copy link
Copy Markdown
Member

This feels like a change for 0.7 instead of 0.6.1 because it will break code. I'm not opposed to moving forward with a 0.7 release and skipping a 0.6.1 release.

Comment thread pvlib/pvsystem.py Outdated
@cwhanse

cwhanse commented Oct 19, 2018

Copy link
Copy Markdown
Member Author

OK with me if we leave this for 0.7. It's an enhancement for consistency in the function level APIs, not a bug fix.

Comment thread pvlib/pvsystem.py Outdated
@cwhanse

cwhanse commented Oct 24, 2018

Copy link
Copy Markdown
Member Author

OK, leaving this PR without whatsnew changes for 0.7

@wholmgren wholmgren added this to the 0.7.0 milestone Nov 3, 2018
@wholmgren wholmgren added the api label Nov 3, 2018
@cwhanse cwhanse closed this Feb 4, 2019
@cwhanse cwhanse deleted the eff_irrad_units branch February 4, 2019 16:50
@wholmgren

Copy link
Copy Markdown
Member

@cwhanse do you no longer want to change the units? If so, we should close the corresponding issue. If not, perhaps we should merge this and make 0.7 the next release. I also have no objections to leaving it open during a 0.6.2 cycle.

@cwhanse

cwhanse commented Feb 5, 2019

Copy link
Copy Markdown
Member Author

I still want to change the units. I was doing some housecleaning in my fork and deleted this branch without realizing that it is connected to this PR.

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

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants