Release 0.12.6 #132
Release 0.12.6 #132
Conversation
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## master #132 +/- ##
==========================================
- Coverage 67.46% 67.22% -0.25%
==========================================
Files 11 11
Lines 1675 1681 +6
==========================================
Hits 1130 1130
- Misses 545 551 +6 ☔ View full report in Codecov by Sentry. |
geofetch/geofetch.py
Outdated
| "To download raw data You must first install the sratoolkit, with prefetch in your PATH." | ||
| " Installation instruction: http://geofetch.databio.org/en/latest/install/" | ||
| ) | ||
| if os.name == "nt": |
There was a problem hiding this comment.
So if its windows, we don't prefetch, and just warn them instead of exiting?
There was a problem hiding this comment.
yes, because I don't know how to check if prefetch exist in windows
There was a problem hiding this comment.
Could we use where? Its available on windows XP and above it seems: https://stackoverflow.com/questions/304319/is-there-an-equivalent-of-which-on-the-windows-command-line
| raise e | ||
| if os.name == "nt": | ||
| _LOGGER.error(f"{e}") | ||
| raise OSError( |
There was a problem hiding this comment.
Should we raise an error here? I feel like we can just check for the wget in the beginning and handle it from there, no?
There was a problem hiding this comment.
Could you try to execute wget --version at the start and mark it unavailable if it raises an error?
In Powershell wget is aliased to Invoke-WebRequest so if you type wget, something will happen but it's not really wget (different command line args etc.) I don't know if Python will cause Powershell commands to be executed or not, but if it does, that could also cause confusion.
There was a problem hiding this comment.
Do you know if any of this is a problem when using WSL? I thought WSL leveled the playing field when it came to posix compatibility with windows. Unfortunately I don't have immediate access to a Windows machine to test some of these things.
But I kind of agree with @pedro-w in that it should be pretty easy to do an initial check at the beginning to see if wget is available at all and then raise errors/move forward from there
There was a problem hiding this comment.
It does appear to work for me under WSL but I only ran one command and saw that it started downloading files (comment)
... but WSL could be one of a number of distros and might not have wget installed (same as Linux, actually)
Updated windows support