euroargodev / argodmqc_owc

Argo float salinity calibration software
Apache License 2.0
10 stars 3 forks source link

Refactor find_besthist #27

Open kamwal opened 4 years ago

kamwal commented 4 years ago

Currently, the find_besthist function requires 18 arguments to run. This is far too many, looks a bit messy, and can make using the function rather difficult.

I suggest taking some of the arguments out and placing them in a class, especially where the same class can be used for differing data.

Create a class for containing float data. This class can contain the latitudes, longitudes, z vales, and dates of different data points. We can either create two objects (one being the float, and the second being all the historical points), or we can create an array of objects, where each object is a single data point. This class could also contain some functions that are exclusively used on the data (i.e. calculating potential vorticity).

We could also create a class for calculating the ellipse that inherits from the other class to remove even more arguments.

I also think we should just pass in the struct containing all the configuration variables into the function, rather than dropping them in one by one.

Doing all the above will reduce the argument count from 18 to 3. I would be much more manageable.

gmaze commented 4 years ago

Create a class for containing float data. This class can contain the latitudes, longitudes, z vales, and dates of different data points. We can either create two objects (one being the float, and the second being all the historical points), or we can create an array of objects, where each object is a single data point. This class could also contain some functions that are exclusively used on the data (i.e. calculating potential vorticity).

This could be made using the www.github.com/euroargodev/argopy data model that comes with this kind of functionalities already

edsmall-bodc commented 4 years ago

@gmaze That's a great idea. Is it the ArgoDataFetcher class you are thinking of in particular? Correct me if I'm wrong, but it looks like we can collect float data from a square region by specifying latitude and longitude, or using WMO boxes?

If that's correct, we can actually get rid of the WMO boxes too, and fetch all our data using this, as long as we can remove the profile currently being analysed from the fetched data set. Shouldn't be hard, as it looks like this fetches the profile names too.

The only issue I am foreseeing is that this only fetches argo data, not CTD/Bottle data too?

gmaze commented 4 years ago

With the ArgoDataFetcher, you can retrieve data from the Argo reference dataset (last 2019V03 version available in the upcoming days) The goal of this feature was precisely to make it more simple to request reference data from QC software like OWC ! And yes, it's trivial to make sure the profile currently analysed is not part of it.

The WMO boxes are a non sense in a modern software framework.

And yes CTD/bottle data are not available now, since this would require some kind of authentication system that we are currently working on.

Have a look to: https://github.com/euroargodev/erddap_usecases/blob/master/examples/Example-Standard-02.ipynb, and even run it online with Binder.

edsmall-bodc commented 4 years ago

@gmaze I see there are some plotting capabilities as well - I think we should chat at some point about the current OWC output to see if any of your plotting routines can be used here?

gmaze commented 4 years ago

not yet very developed since it's not the core focus of the library (which is to fetch and manipulate argo data), but working on notebooks and visualisation examples yes, surely a lot of overlap here

edsmall-bodc commented 4 years ago

I guess, for now, I can make the plotting routines that exist in the matlab version in python, with a view to potentially make an argo plotting package for all argo focused plotting routines to exist