Skip to content

iotools component for maccrad #274

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

Closed
wants to merge 15 commits into from
Closed

iotools component for maccrad #274

wants to merge 15 commits into from

Conversation

dacoex
Copy link
Contributor

@dacoex dacoex commented Dec 2, 2016

See: #272 for reason.

Previous discussion in #271

@wholmgren I hope I got this git thing right... But anyway, we are in a separate branch than master.

@dacoex dacoex changed the title Io for maccrad iotools component for maccrad Dec 2, 2016
@wholmgren
Copy link
Member

Thanks. Some general comments:

I'd prefer that we tackle one format per pull request. This will make each one go faster and be less likely to become stale and neglected.

It looks like you're duplicating the new code in the pvlib-python/io branch instead of adding to it and that's what led to the merge conflict. To build on what's already in the pvlib-python/io branch, you'll need to fetch pvlib-python's io branch and then rebase your changes on it (or merge it into your branch). Depending on your git experience, it might be easier to make a fresh clone, checkout the io branch, and copy over the files related to macc or pvsyst.

There are probably too many data files and they're probably too long. We only want to include the bare minimum of data required to test the code and to let users run an example or two.

The iotools/iotools.py module should be renamed iotools/util.py to avoid confusion.

I don't think we should create a dependency on the timezonefinder package. That's probably best left to users to implement themselves, if desired.

@dacoex dacoex closed this Dec 5, 2016
This was referenced Dec 5, 2016
@mikofski mikofski mentioned this pull request Jan 7, 2020
8 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants