-
Notifications
You must be signed in to change notification settings - Fork 23
Support PropertyField location after server changes #2322
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
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #2322 +/- ##
=======================================
Coverage ? 84.07%
=======================================
Files ? 91
Lines ? 10701
Branches ? 0
=======================================
Hits ? 8997
Misses ? 1704
Partials ? 0 |
sync successful CI |
3a5d304
to
454ae04
Compare
src/ansys/dpf/core/property_field.py
Outdated
) | ||
self._field_definition_instance = None | ||
if meets_version(self._server.version, "11.0"): | ||
self._field_definition_instance = self._load_field_definition() |
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.
why doing it at init and not lazy anymore? It can have non negligeable impact on performance
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.
Yes, you are right, let's see if the tests are not impacted by this change. I was replicating the behavior of the Field, but you are right on the performance point.
return field_grpcapi._get_stub(client).Create(request) | ||
|
||
@staticmethod | ||
def csproperty_field_new_location_on_client(client, numEntities, data_size, location): |
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.
why did we need a new API here, couldn't we do field_def_new and then set_location?
Adter the last server changes in v11, the
PropertyField
location is defined on itsFieldDefinition
and it is independent from the location of itsScoping
. Changes are made to the setters and getters to support this.