-
Notifications
You must be signed in to change notification settings - Fork 74
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
Field.__call__ has inconsistent returns: use dictionaries #60
Comments
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, ... 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... |
I like the idea of having a data class or just some standard structure like a The use of dictionaries for this is fairly common in my experience. For example, |
@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. |
The
__call__
method has different returns acrossField
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 inField.mesh
The text was updated successfully, but these errors were encountered: