diffsync icon indicating copy to clipboard operation
diffsync copied to clipboard

Remove `diffsync` field from DiffSyncModel

Open jamesharr opened this issue 1 year ago • 4 comments

Environment

  • DiffSync version: 1.9.0

Proposed Functionality

  • Remove the diffsync field from DiffSyncModel
  • Add diffsync to DiffSyncModel.update and DiffSyncModel.create method signatures

There are a few reasons behind my thinking: 1) It makes the API a bit more consistent 2) It might make it easier for Python to run GC if there's no/fewer cyclical references between objects 3) I didn't see a lot of references to this field outside of create/update/delete

This is likely appropriate for diffsync v2 #232 since it breaks the API.

Other feature requests like #255 may benefit from this change since it would mean there is no assumption that DiffSyncModel has a reference to a given DIffSync

Use Case

class DiffSyncModel(BaseModel):
    @classmethod
    def create(cls, diffsync: "DiffSync", ids: Dict, attrs: Dict) -> Optional[Self]:
        # Method signature remains the same
        ...

    def update(self, diffsync: "DiffSync", attrs: Dict):
        ...

    def delete(self, diffsync: "DiffSync"):
        ...

jamesharr avatar Nov 20 '23 04:11 jamesharr