Skip to content

Remove automatic MIDC variable mapping #721

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 5 commits into from
May 14, 2019
Merged

Remove automatic MIDC variable mapping #721

merged 5 commits into from
May 14, 2019

Conversation

lboeman
Copy link
Contributor

@lboeman lboeman commented May 13, 2019 •
edited by wholmgren
Loading

  • Closes NREL MIDC parser variable mapping makes too many assumptions. #720
  • 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.

@lboeman lboeman changed the title Midc mapping Remove automatic MIDC variable mapping May 13, 2019
@wholmgren wholmgren added api bug io solarfx2 DOE SETO Solar Forecasting 2 / Solar Forecast Arbiter labels May 14, 2019
@wholmgren wholmgren added this to the 0.6.2 milestone May 14, 2019
wholmgren
wholmgren approved these changes May 14, 2019
View reviewed changes
Copy link
Member

@wholmgren wholmgren left a comment •
edited
Loading

Choose a reason for hiding this comment

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

will merge in a day or so if there are no concerns.

cwhanse
cwhanse approved these changes May 14, 2019
View reviewed changes
Copy link
Member

@cwhanse cwhanse left a comment

Choose a reason for hiding this comment

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

I agree with this approach. I'll pass this observation to Manajit and see if they are open to adopting a more consistent technique for labeling data, might help.

@wholmgren wholmgren merged commit 15b1cbd into pvlib:master May 14, 2019
@lboeman lboeman deleted the midc_mapping branch May 14, 2019 16:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@wholmgren wholmgren wholmgren approved these changes

@cwhanse cwhanse cwhanse approved these changes

Assignees
No one assigned
Labels
api bug io solarfx2 DOE SETO Solar Forecasting 2 / Solar Forecast Arbiter
Projects
None yet
Milestone
0.6.2
Development

Successfully merging this pull request may close these issues.

NREL MIDC parser variable mapping makes too many assumptions.
3 participants
@lboeman @wholmgren @cwhanse