-
Notifications
You must be signed in to change notification settings - Fork 4
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
[Prep for NEAT-48] Refactor: _DMSExporter #321
Conversation
…dule-part2 # Conflicts: # cognite/neat/rules/issues/base.py # cognite/neat/rules/issues/dms.py # cognite/neat/rules/issues/fileread.py # cognite/neat/rules/issues/importing.py # cognite/neat/rules/issues/spreadsheet.py # cognite/neat/rules/issues/spreadsheet_file.py # tests/tests_unit/rules/test_issues/test_issues_metatests.py
…tion-list # Conflicts: # cognite/neat/rules/issues/dms.py
data_model.space = used_spaces.pop() | ||
spaces = dm.SpaceApplyList([dm.SpaceApply(space=data_model.space)]) | ||
else: | ||
spaces = dm.SpaceApplyList([metadata.as_space()] + [dm.SpaceApply(space=space) for space in used_spaces]) |
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.
doesn't DMSMetadata have field space
already, why calling method?
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.
To get the space object
else: | ||
type_cls = dm.DirectRelation | ||
|
||
prop_name = to_camel(prop.container_property) if self.standardize_casing else prop.container_property |
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.
For time being this is fine, but we already have under SheetEntity defined option to add name
field, in case for example when property_ and class_ (i.e. ids) are human unreadable, which is the case for one of our clients.
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.
Not sure if I understand this. Are you suggesting a better way to handle properties?
# Conflicts: # tests/tests_unit/rules/test_models/test_dms_architect_rules.py
# Conflicts: # cognite/neat/rules/models/_rules/dms_architect_rules.py # tests/tests_unit/rules/test_models/test_dms_architect_rules.py
This is refactoring the
_DMSExporter
by gathering and moving out the methods for creating the containers, views, and spaces.This is necessary as with adding filter to the views the code became very messy. Dragging this out as its own PR to separate the refactoring from actually adding the feature.