Skip to content
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

Download NY Head model files automatically if missing #192

Merged
merged 8 commits into from Apr 12, 2023

Conversation

torbjone
Copy link
Collaborator

@torbjone torbjone commented Apr 11, 2023

Switch to automatic downloading of New York Head model, if file location is not given.

Closes #193

else:
print("Exiting program ...")
sys.exit()
except Exception:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this should capture URLError (as in URLError: <urlopen error [Errno -2] Name or service not known> or URLError: <urlopen error [Errno 8] nodename nor servname provided, or not known>), otherwise this looks good!

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think URLError is a built-in error in python though, are you sure we should do this? I considered catching both PermissionError and URLErrors, but originally decided against it, since in the end I did not supply any more information than what was already effectively given in the error name.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

URLError is raised by urllib which is part of the standard distribution. I think it is fine to capture it as well as PermissionError then. Having consecutive excepts should be ok, according to the docs:

import sys
try:
    f = open('myfile.txt')
    s = f.readline()
    i = int(s.strip())
except OSError as err:
    print("OS error:", err)
except ValueError:
    print("Could not convert data to an integer.")
except Exception as err:
    print(f"Unexpected {err=}, {type(err)=}")
    raise

Copy link
Contributor

@espenhgn espenhgn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you also bump the version number in LFPykit/version.py to 0.5.1?

else:
print("Exiting program ...")
sys.exit()
except Exception:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

URLError is raised by urllib which is part of the standard distribution. I think it is fine to capture it as well as PermissionError then. Having consecutive excepts should be ok, according to the docs:

import sys
try:
    f = open('myfile.txt')
    s = f.readline()
    i = int(s.strip())
except OSError as err:
    print("OS error:", err)
except ValueError:
    print("Could not convert data to an integer.")
except Exception as err:
    print(f"Unexpected {err=}, {type(err)=}")
    raise

@espenhgn espenhgn changed the title Input fix Download NY Head model files automatically if missing Apr 11, 2023
@torbjone
Copy link
Collaborator Author

Does it look good now?

Copy link
Contributor

@espenhgn espenhgn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Excellent!

@espenhgn espenhgn merged commit 0c43f11 into LFPy:master Apr 12, 2023
9 checks passed
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.

Download NYheadmodel files automatically
2 participants