-
Notifications
You must be signed in to change notification settings - Fork 75
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
UW sounding regions #199
UW sounding regions #199
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Minor things.
siphon/simplewebservice/wyoming.py
Outdated
try: | ||
region = regions[region.lower()] | ||
except KeyError: | ||
raise(KeyError('Invalid region {}. Valid regions are ' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
raise
is not a function. Also, the indentation should line up with the '
in KeyError('
.
siphon/simplewebservice/wyoming.py
Outdated
region (optional) : str | ||
Region for the sounding data. Valid regions are `North America`, | ||
`South America`, `South Pacific`, `New Zealand`, `Antarctica`, | ||
`Arctic`, `Europe`, `Africa`, `Southeast Asia`, `Mideast` Defaults |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Period after Mideast
. Also, should probably replace backticks with single quotes...I think.
4b0dbb9
to
8dfdcbe
Compare
Interestingly enough, no matter what the region is, you can get data for any station anywhere. I believe this is a fluke in the web API as I verified the behavior in the browser. Selecting a region changes the map, but doesn't appear to actually limit the station search area in the database. We should probably ping Larry on this. |
8dfdcbe
to
ceb653d
Compare
siphon/simplewebservice/wyoming.py
Outdated
try: | ||
region = regions[region.lower()] | ||
except KeyError: | ||
raise KeyError("Invalid region {}. Valid regions are 'North America'" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should probably raise a ValueError
instead of KeyError
, since the error is that the user passed in a bad value.
d101e02
to
5989bbd
Compare
5989bbd
to
1a0b21e
Compare
Larry says they do not use region except for the map on the website, so we don't need the ability to pass it. I've just defaulted to naconf and removed the ability to specify a region. |
1a0b21e
to
3b19cec
Compare
Closes #176 and improves the region functionality. Users can specify human readable names instead of the abbreviations (which aren't listed anywhere I've found). The region will also be case insensitive. Adds a test for making sure the region is valid or a helpful
KeyError
is raised.