Add map_variables parameter to read_tmy3 function#1623
Add map_variables parameter to read_tmy3 function#1623kandersolar merged 50 commits intopvlib:mainfrom
Conversation
|
Hi @ooprathamm Thank you very much for tackling this issue 😀 There's a couple of formatting issues (stickler). If you can solve those, then I'll go ahead and give a detailed review. If you have any issues feel free to post here and we'll get it sorted! Also, if you update the description with the issue number (closes #xxxx), then GitHub will automatically link the two. |
|
@ooprathamm Mapping of variables using the |
ooprathamm
changed the title
|
@ooprathamm are you still working on this issue? Again, don't hesitate to reach out for help if you are stuck. |
AdamRJensen
left a comment
AdamRJensen
left a comment
There was a problem hiding this comment.
It's looking very good so far, well done!
Besides the few comments I've added, there needs to be added one or two tests that checks if the variable mapping does what it is supposed to. The tests for the TMY3 functions are located here.
You can check out this test from the surfrad functions for inspiration.
It would also be ideal to have one test case that checks if the warning is raised properly, see this test case for how to do that.
Co-authored-by: Adam R. Jensen <39184289+AdamRJensen@users.noreply.github.com>
Co-authored-by: Adam R. Jensen <39184289+AdamRJensen@users.noreply.github.com>
Co-authored-by: Adam R. Jensen <39184289+AdamRJensen@users.noreply.github.com>
Co-authored-by: Adam R. Jensen <39184289+AdamRJensen@users.noreply.github.com>
AdamRJensen
added
remote-data
Co-authored-by: Kevin Anderson <kevin.anderso@gmail.com>
Co-authored-by: Kevin Anderson <kevin.anderso@gmail.com>
Co-authored-by: Kevin Anderson <kevin.anderso@gmail.com>
kandersolar
left a comment
kandersolar
left a comment
There was a problem hiding this comment.
I think I like how read_tmy3 looks now :)
I think this PR will be ready after cleaning up the deprecation warnings emitted elsewhere in pvlib: some tests in test_location.py and test_soiling.py (https://github.com/pvlib/pvlib-python/actions/runs/5047873887/jobs/9055325947?pr=1623#step:9:66), plus several pages in the user guide and gallery. Some notebooks too, but I suggest leaving those to continue collecting dust and falling out of date. $ git grep read_tmy3 is helpful in tracking down all the places read_tmy3 gets used.
kandersolar
left a comment
kandersolar
left a comment
There was a problem hiding this comment.
LGTM pending the stickler stuff
|
Thanks @ooprathamm and @AdamRJensen! What a big job this turned out to be. |
4 participants
[ ] Updates entries indocs/sphinx/source/referencefor API changes.docs/sphinx/source/whatsnewfor 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`).remote-data) and Milestone are assigned to the Pull Request and linked Issue.