GeoStat-Framework / GSTools

GSTools - A geostatistical toolbox: random fields, variogram estimation, covariance models, kriging and much more
https://geostat-framework.org
GNU Lesser General Public License v3.0
567 stars 74 forks source link

Field.__call__ has inconsistent returns: use dictionaries #60

Closed banesullivan closed 4 years ago

banesullivan commented 4 years ago

The __call__ method has different returns across Field subclasses... this is really messy. Would it be of interest to return a dictionary of numpy arrays in all cases?

For example, the krige classes return the field and the variance while all other field subclasses only return the field.

Using a dictionary with named arrays would help the user knows what's what when the arrays are returned and would be a lot cleaner, safer when capturing the output from different Field subclasses, and also make it seamless to add those arrays to either Meshio or PyVista meshes in Field.mesh

LSchueler commented 4 years ago

I completely agree with you. I am currently working on a new interface for the variogram estimation.

I also stumbled upon the very messy handling of all the different fields and the return values. Therefore I introduced a new "data class" which is handed around and saves stuff like the position of the field values, all the different fields, ... But the class will work very similar to a dictionary.

I have a totally unrelated dead line on Wednesday, thus I don't know how much I will get done, but I hope, that I can put up a PR soon in order to discuss in which direction to take things.

I'm still not sure whether to inherent all the different field classes from a data class or if they should have them as attributes. Let's see...

banesullivan commented 4 years ago

I like the idea of having a data class or just some standard structure like a dict to hold all of the data. Ping me when you are able to get a PR going as I'd be happy to help on that where I can

The use of dictionaries for this is fairly common in my experience. For example, SimPEG uses dictionaries to hold numpy arrays for data and models

LSchueler commented 4 years ago

@banesullivan I just created a PR #65, where we can discuss things. I'd be more than glad to hear what you think about my suggestions. I'll close this issue and we can continue the discussion in the PR.