Closed fernando-aristizabal closed 1 year ago
Thanks for the PR! Overall, looks good to me. I added a few minor comments.
Hi @cheginit - Thanks for the quick review! I'm just wondering if your comments are tagged as pending on your end?
Here is a discussion on the matter that might be relevant.
I sent the previous comment before finishing my comments on the code. I was trying to figure out the issues with WFS and WMS. Can you see them now?
The WFS issue seems to be related to an issue with the web service. In the dev version of pygeoogc
I made a few changes to catch some errors, so you'd have to install it from git to test it. The NFHL service returns this error: <ExceptionText><![CDATA[OutputFormat 'text/xml' not supported.]]></ExceptionText>
. So it seems to be some kind of misconfiguration on their part.
For WMS, the service requires providing style
, so this seems to work:
wms = WMS(
"https://hazards.fema.gov/nfhl/services/public/NFHLWMS/MapServer/WMSServer",
layers=0,
outformat="image/tiff",
crs=4269,
validation=False,
)
fema_dict = wms.getmap_bybox((-73.42, 43.28, -72.9, 43.52), 1e3, box_crs=4269, kwargs={"styles": "default"})
The WFS issue seems to be related to an issue with the web service. In the dev version of
pygeoogc
I made a few changes to catch some errors, so you'd have to install it from git to test it. The NFHL service returns this error:<ExceptionText><![CDATA[OutputFormat 'text/xml' not supported.]]></ExceptionText>
. So it seems to be some kind of misconfiguration on their part.For WMS, the service requires providing
style
, so this seems to work:wms = WMS( "https://hazards.fema.gov/nfhl/services/public/NFHLWMS/MapServer/WMSServer", layers=0, outformat="image/tiff", crs=4269, validation=False, ) fema_dict = wms.getmap_bybox((-73.42, 43.28, -72.9, 43.52), 1e3, box_crs=4269, kwargs={"styles": "default"})
Thanks for looking into this! The WMS now works for me given the style argument.
I think the only that's left is removing the commented lines, right?
Yes that's right. I've pushed up the latest with the lines commented out and some exceptions renamed. Unfortunately, the tests aren't currently passing due a possibly broader issue.
I'm currently getting 62 failing tests within pygeohydro in this branch and the main branch as well. Almost all of them appear to be related to:
FAILED tests/test_pygeohydro.py::test_sensorthings - async_retriever.exceptions.InputValueError: Given request_kwds (params) is invalid. Valid options are:
Detailing the error on a specific test reveals:
not_found = [p for kwds in request_kwds for p in kwds if p not in session_kwds]
if not_found:
invalids = ", ".join(not_found)
E raise InputValueError(f"request_kwds ({invalids})", list(session_kwds))
E async_retriever.exceptions.InputValueError: Given request_kwds (params) is invalid. Valid options are:
E self
E method
E str_or_url
E expire_after
E kwargs
.nox/tests-3-8/lib/python3.8/site-packages/async_retriever/_utils.py:208: InputValueError
The issue was related to a breaking change in one of the dependencies of async-retriever
. I just pushed a fix. Can you please try running nox
again?
Thanks for getting that addressed. My latest commit passes all tests. It should be ready to merge!
Awesome, thanks for taking the time to address the issue and your contribution!
The following is a draft/WIP PR that adds support for FEMA's National Flood Hazard Layer via their ArcGISRestful APIs. I selected the ArcGIS services in favor of the WFS or WMS options due to greater optionality and reliability. Please see some of the issues with WFS and WMS observed below.
Example Usage
Issues
WFS
PyGeoOGC: Retrieve Data from RESTful, WMS, and WFS Services Notebook
yields the following error:
changing layer parameter to
"NFHL:Base_Flood_Elevations"
yields:PyGeoUtils: Utilities for (Geo)JSON and (Geo)TIFF Conversion Notebook
yields:
WMS
Trying WMS gives this issue:
Note
Connecting to both the WFS and WMS via QGIS both resolve in issues as well so this may not be HyRiver specific.
To-Do
Update
pygeoogc
with additional ServiceURLs.nox
passes.pre-commit run --all-files
HISTORY.rst
.