-
Notifications
You must be signed in to change notification settings - Fork 33
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
Create a geoSeries class as a subclass of Series. Same for MultipleSeries/EnsembleSeries. #326
Comments
I like the GeoSeries idea, and it poses no problem to extend it to GeoMultipleSeries (child of MultipleSeries). The only thing to work out is the requirements of a This poses no particular problem and it should not break anything (famous last words). However, we do need to decide where this falls in terms of development. I'd like to do this after pandas integration is complete, what do you think? |
Definitely after the pandas integration is complete. Since |
TODO
MultipleGeoSeries
EnsembleGeoSeries
MultipleEnsembleGeoSeries
|
For GeoSeries
|
For GeoSeries, the following methods should be moved:
Map and dashboard implemented in 9e1322e. mapNearRecord requires MulitpleGeoSeries |
Update: by popular demand, GeoSeries now includes as optional arguments elevation (float), depth (array), SensorType(str), ObservationType(str). |
geoseries done as of #420. Now onto the rest... |
One of the mapping is missing from this list: Should we attach it to Either way, needs to be added to the to-do list. |
EnsembleGeoSeries:
|
EnsembleSeries helper function Needs four things:
A: For Series, two main cases to handle:
B: For GeoSeries, four main cases to handle: Same as above, only now depth can be present in the GeoSeries object.
For GeoSeries also accept a depth unit argument and, if depth is being pulled from the GeoSeries, check that the units match (assuming GeoSeries has an associated depth unit). Check list:
Note: Update the lipdutils function that does this with the notebook function and use that for the machinery (do error handling and whatnot separately). Second Note: Need to think about extrapolation handling at some point. |
@alexkjames I think your latest pull request took care of the majority of the issues (I checked the boxes). One thing: the doctoring example use made up time series, which is fine for this. But one thing that may be missing: we are not checking that all the lat/lon are the same; we are imposing the first one. Is that what we want to do? |
closing this as only #594 is left, and it not does hinder functionality |
Is your feature request related to a problem? Please describe.
Adding location is important for some applications but not others. Current implementation only allows geo information through
LipdSeries
. The development branch has an implementation ofSeries
that would takelat/lon
arg as optional since most of our methods do not require geolocation. For the methods that do, we return an error if lat/lon are not specified, which is not ideal.Describe the solution you'd like
We consider two solutions:
Series
, calledGeoSeries
that would require the use of lat/lon for mapping functionaries but reuse the existing functions inSeries
class for all other applications. This schema would need to be extended toGeoMultipleSeries
to allow for EOF/PCA analysis.The text was updated successfully, but these errors were encountered: