-
Notifications
You must be signed in to change notification settings - Fork 0
Block models v2 #2
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
Conversation
| @classmethod | ||
| def deserialize(cls, value, trusted=False, strict=False, assert_valid=False, **kwargs): | ||
| schema = value.pop("schema", "") | ||
| def __lookup_class(cls, schema): |
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 fixes getting useless errors when a schema is not found.
| ContinuousColormap, | ||
| NumericAttribute, | ||
| StringAttribute, | ||
| VectorAttribute, |
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.
Direct imports mean you get errors earlier when changing things, helping to avoid bugs in production.
| disable = "consider-using-f-string" | ||
| disable = [ | ||
| "consider-using-f-string", | ||
| "no-name-in-module", |
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.
PyLint kept marking this error when there was nothing wrong.
grantroch
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.
Very minor things
ThomasMatern
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.
probably a merge conflict: test_doc_example.py should have been deleted but it's an empty file on this branch
|
I have made a bunch more changes to this, simplifying it down to one new top-level element with what I think is a better layout with in. This keeps common fields out of components and reduces the total number of classes. I have also fleshed out the documentation so everything should make sense now. |
Replaces the block model classes with new ones that should be simpler, faster, and easier to understand. There is now a single top-level element with switchable gridding and optional sub-blocks, giving fewer smaller classes.
Also cleaned up some related and unrelated code and fixed a minor bug.