-
Notifications
You must be signed in to change notification settings - Fork 64
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
Complete Refactor of Ulmo #109
Comments
I might be able to contribute, depending on the timing, etc. If nothing else, by posting this comment I'm adding myself to the notifications on discussions on this issue. Quick question/comment: Are you considering using GeoPandas when the DataFrame has a spatial component? I've used GeoDataFrames a bit, but not enough to have a solid opinion regarding its maturity. |
I'm actually researching spatial indexes right now. I'm leaning away from GeoPandas right now since it has dependencies on shapely and fiona and hence GEOS and GDAL which are a pain to install easily cross platform. I'm considering having a geometry column that is just an array of coords (Point/Line/Poly) to enable some simple bbox filtering. |
Makes sense. |
@emiliom @wilsaj @nathanhilbert @cameronbracken So do folks have any preference between these two api approaches. I'm leaning towards b) but wanted to get some input. a) Flat API. You pass service name and dataset name and any other parameters to each call: stations = ulmo.get_features('usgs-nwis', 'iv', state='TX') This makes the api simpler and clearer to use but potentially less flexible. i.e not all services have something analogous to get_features (see usgs.eddn, we would have to raise a 'Not Implemented' on that). It is also a bit more verbose to type. The API would have to cover all the main use cases. b) load a plugin by specifying service and dataset and then use that. nwis = ulmo.load_service('usgs-nwis', 'iv') This api is more flexible since each plugin could define its own api, we would have some base classes to maintain consistency for similar plugins (i.e. timeseries, raster etc) to keep the api reasonably consistent across plugins. |
how would you use b) with the CUAHSI WaterOneFlow / WaterML web services? would it be something like: cuahsi = ulmo.load_service('cuahsi-his', 'http://hydroportal.cuahsi.org/GLEON_Sunapee/cuahsi_1_1.asmx') or would you consider the 'CUAHSI HIS Central' as a service and each of the HydroServers as a dataset? |
About the Python 3 support, I suggest that you can remove the dependency on suds, according to my knowledge the suds package only exists for Python 2 and it's not really used by ulmo except for the CUAHSI WaterOneFlow. One actively maintained replacement package to consider is PySimpleSOAP: https://pypi.python.org/pypi/PySimpleSOAP |
The api I am considering is: ulmo.list_services -> Gets a list of services available to ulmo. (i.e. nwis, cdec etc) So I think 'CUAHSI HIS Central' would be the service and each HydroServer would be a dataset. The other change I'm considering is moving each service into its own repo as an extension, potentially with its own maintainer. The pattern would be similar to the system the 'flask' package uses. i.e. we would have separate python ulmo (that has common functions and base classes) and ulmo_cuahsi, ulmo_nwis, etc. Advantages:
Disadvantage: The main disadvantage of this approach is you would know have to install several modules to get full functionality, but I guess we could make a meta package that pulled in all supported plugins. I don't have the bandwidth to support all the data sources so distributing the load would help out enormously. @jirikadlec2 would you be available convert from suds to PySimpleSOAP? I currently don't use HIS services much. |
I'm going to experiment with the approach of having a package called 'ulmo-common' and converting the services to a extensions named 'ulmo-extensionname' |
I'm planning a complete refactor of ulmo with the following (major) target features:
I will probably tag the release 1.0 to indicate that it will break backwards compatibility.
If folks are interested in contributing to the refactor or wish to discuss these changes in more detail. Please comment on this issue.
The text was updated successfully, but these errors were encountered: