[PY-622][PY-626][PY-665] Import Properties (create/update) while importing annotations #759
Conversation
| @@ -405,13 +405,13 @@ class Property: | |||
| name: str | |||
|
|
|||
| # Type of the property | |||
| type: str | |||
| type: PropertyType | |||
There was a problem hiding this comment.
It wasn't you who did this, but changing this block to a triple quote block instead would be a decent low hanging fruit improvement.
There was a problem hiding this comment.
Do you mean something like this:
type: """PropertyType"""
| @@ -267,6 +274,159 @@ def _resolve_annotation_classes( | |||
| return local_classes_not_in_dataset, local_classes_not_in_team | |||
|
|
|||
|
|
|||
| def _import_properties( | |||
| metadata_path: Union[Path, bool], | |||
There was a problem hiding this comment.
With this function, I'd (like in the other PR I reviewed), say break this down. I haven't counted the McCabe complexity of this, but it looks like it's quite high. We removed the requirement for lower than 10 complexity because of some older inherited code, but it would be good to apply a higher standard for these lints on this level.
Also, same note on the function names. These can be more explanatory, and refactored with no difficulty.
If you break this down, no need to rewrite tests if it's already adequately covered, as this just becomes social testing of all.
There was a problem hiding this comment.
I believe we have some tickets on E2E test on properties import - so testing this function properly should be covered in that.
For the function names, what would you prefer? some examples:
_import_properties->_create_update_properties? - as it Creates/Updates missing/mismatched properties from annotation & metadata.json file to team-properties._update_payload_with_properties->_update_annotation_payload_with_properties? - as it updates the annotation import API payload data with annotation-properties data
What are your thoughts on it?
* handling properties data while importing annotations * add tests for _handle_annotation_data * update importer - add annotations_properties in payload of import annotations * update importer - rm _handle_properties code * fix test * update test * fix indent * add the missing return value * fix issues with multi_select create_property
saurbhc
changed the title
saurbhc
requested review from
rslota and
AndriiKlymchuk
and removed request for
Nathanjp91 and
owencjones
saurbhc
changed the title
…d value field (#769) * update nested value changes for property-value objects * fix tests + fix linting
Problem
Solution
Implement
_import_propertiesbefore_import_annotations& add those properties in import annotations API payload.Changelog
Implement Import Properties (create/update) while importing annotations