-
Notifications
You must be signed in to change notification settings - Fork 23
Regular 3D grid typed object #164
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
Regular 3D grid typed object #164
Conversation
Use this when downloading category data.
The added methods upload_category_table/upload_category_dataframe extracts the indices and lookup table, and uploads both. This also adds support for specifying the table format or formats to allow when saving or uploading data.
…pyarrow) is more efficient as internally uploading is taking advantage of the dictionary encoded representation.
… for ServiceManager/ServiceManagerWidget. Also rename EvoContext to StaticContext.
RyanMillerSeequent
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
rileyhowley-seequent
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is great work @BenLewis-Seequent and will be very beneficial for users. I just had a couple of questions but nothing blocking.
wordsworthc
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this is a good start, but I wonder how this could evolve over time.
There are a few areas where I am overwhelmed by the sheer volume of code, and I am left wondering if there is a cleaner solution. The _BaseObject, for example, seems to duplicate most of the functionality of DownloadedObject (and yes, it adds a little new functionality for uploading too). I've also made a specific suggestion to clean up usage of the SchemaProperty descriptor (I like the PoC!).
Is this intended as a preview release? I have a few ideas it would be nice to try out and share in the new year sometime, but I would be very worried about breaking the library API
I have split out some of the logic from
Agree, I just don't know how a preview release would work though, as other changes may well want to be released well we still evaluate this. A possibly is to somehow mark this package as a "preview" package, other libraries sometimes have a package named "preview". So the import for this could be |
|
To reduce the scope of this PR, I removed support for loading and saving non-attribute arrays/tables. These will be needed in the future, but can be added when needed. |
wordsworthc
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Happy New Year 🎉
I must apologize, I have been swamped with other initiatives and have not yet found the time to provide a qualitative re-review of your pull request. Unfortunately I do not see myself having the chance in the near future either.
Because I know you, and as we have discussed in person, I trust that you have addressed my earlier comments thoroughly and sensibly, and it is on that basis that I am adding my approval here.
I agree with you that it would be nice to be able to mark some of this as experimental, i.e., subject to change at a later date. I think we are somewhat protected by using dev versioning here (0.y.z), although some additional comments in docstrings would go a long way to making it explicit IMO. I will leave it for others to discuss the best way to communicate the specifics here.
Thank you for your significant effort, I believe your work is an invaluable contribution to the future of this SDK 🙂
Co-authored-by: Lex Macdonald <23644189+l-macdonald@users.noreply.github.com>
Description
This introduces "typed objects", using regular 3D grid as an example.
The existing interfaces in the
evo-objectsSDK is around handling Geoscience Objects generically with respect to the object type. Though in a lot of use cases, the user of the SDK knows which type of object they want to handle, whether that is a point set, regular 3D grid, etc. Providing classes that are specific to a particular type of object, provides better user experience as the user does not need to know the details about the schemas, and can just use auto complete or refer to the class directly. It also gives the ability to do more validation.Examples
Creating an regular 3D grid:
Download:
Update:
Schema versions
This is designed to gracefully handle multiple major versions of object schemas, through the use of adapters that map the details of the schemas to the class properties. This isn't fully fleshed out yet, but these details are not part of the public(stable) interface.
This also only inspects schema properties upon access, meaning it should be able to handle newer minor versions of the object schemas, as long as the parts of the object that is accessed isn't using values added in the newer versions(i.e. new enum values).
Checklist