Improve portability w/ hdw file download#438
Improve portability w/ hdw file download#438Wtristen wants to merge 2 commits intoSuperDARN:developfrom
Conversation
|
Sorry if I've goofed something up! This is the first pull-request I've ever attempted and I'm generally new to version control. |
carleyjmartin
left a comment
carleyjmartin
left a comment
There was a problem hiding this comment.
Installed and imported no issue, installed hdw files on installation no issue.
Removed hdw files and used get_hdw_files() with no issues also.
For now I would remove the test_hdw file, or you can look into pytest and redo it to look similar to the other test files so it works with pytest. It can also be added as a test within test_utils too if that's easier?
Comments and suggestions are just keeping style consistent and tidying. I use flake8 to check pep8 styling if, which you can install with pip and run like flake8 file.py and it will give you places to check.
Great PR!
| #from subprocess import check_call | ||
|
|
||
| #new imports |
There was a problem hiding this comment.
| #from subprocess import check_call | |
| #new imports |
| # TODO: implement when DSWG starts versioning hardware files | ||
| if version is not None: | ||
| raise Exception("This feature is not implemented yet") | ||
| raise NotImplementedError("This feature is not implemented, Versioned hardware does not exist yet.") #clarified error |
There was a problem hiding this comment.
| raise NotImplementedError("This feature is not implemented, Versioned hardware does not exist yet.") #clarified error | |
| raise NotImplementedError("This feature is not implemented, Versioned hardware does not exist yet.") |
2 participants
Scope
Modified pydarn.utils.superdar_radars.py 'get_hdw_files' to improve code portability between OS platforms.
This does not change hdw files themselves or affect any other part of pydarn. The solution replaces external command-line dependencies curl and unzip in get_hdw_files with native python modules urllib.request and zipfile.
issue:
Resolves issue #402 and anticipated portability bugs with use of curl and string path (replaced with os.path)
Approval
Number of approvals: number of approvals before one can merge the code
Test
matplotlib version:
Note testers: please indicate what version of matplotlib you are using
provide a list or code to test
Code to test was uploaded under test/test_hdw.py. To reproduce the issue, use a platform that does not include unzip or curl or one that does not support '/' paths (like old windows I believe); and ensure your hdw directory is empty.
*Reminder, please check your code is: