Closed LukeJones123 closed 2 years ago
Hi @LukeJones123, this PR looks good. Can you please add a couple of unit tests to verify that the code is doing the right thing. You could put them in:
/test_na_file.py
The usual tests are running fine for me and the solutions looks nice.
Hi Ag. I've added a test now. I thought it would clutter things up in test_na_file.py so I made a separate test module. It uses your 2010b.na file because it has a complicated units string for PV which is not parsed well by default.
@LukeJones123 and @FObersteiner, I think you are discussing this further in #45. Let me know if any action is needed from my end.
@agstephens Luke and I agree that it's not the most elegant solution, but we can both live with it - please go ahead and merge.
Thanks @LukeJones123 @FObersteiner
Pull Request Checklist:
[X] This PR addresses an already opened issue (for bug fixes / features)
[x] Tests for the changes have been added (for bug fixes / features)
[ ] Documentation has been added / updated (for bug fixes / features)
[ ] HISTORY.rst has been updated (with summary of main changes)
[ ] I have added my relevant user information to
AUTHORS.md
What kind of change does this PR introduce?: Feature
Does this PR introduce a breaking change?:
Other information: