PySyft icon indicating copy to clipboard operation
PySyft copied to clipboard

Investigate Removal of SyPrimitive

Open madhavajay opened this issue 4 years ago • 2 comments

Description

We should investigate what would be required to remove SyPrimitive. If we are no longer storing anything on the objects we just need the following: x = 1

  • x_ptr = peer.store(obj=x)
  • AST Dict {type(x):wrapper_type} for handling serde
  • Deleting lots of code

OR

Refactor to a metaclass. https://openmined.slack.com/archives/G0186Q3EAHL/p1613617152209300?thread_ts=1613576044.207200&cid=G0186Q3EAHL

Definition of Done

Investigating if it would work and if so creating the PR that removes all the unneeded code and tests.

madhavajay avatar Feb 17 '21 05:02 madhavajay

A few notes on this @madhavajay :

  • removal of Int, Float and other primitives that are non-iterable is possible 100%, there is no actual need of them if we don't need an id set on them.
  • removal of List, Dict and other primitives that are iterable is possible, but we won't be able to use templating/actually type them. Here a metaclass would be really handy (the testing should remain tho)
  • removal of Iterator I would say it's not possible.

If we implement the client.send(obj) or syft.send(obj, client) we might be able to remove a ton of code, but first we need to be sure that's the desired API (I am totally into it). Are we sure we want to make this refactor for 0.5? I would say it would be nice to release the new API with 0.5 and then take our time to clean the codebase at our own peace to be sure we get it right this time.

tudorcebere avatar Mar 07 '21 08:03 tudorcebere

@tudorcebere Okay I totally agree, since its working lets hold this off till after 0.5 but we should definitely revisit this as I believe at a minimum we can refactor a lot of this common code away and then remove it later if possible.

madhavajay avatar Mar 10 '21 06:03 madhavajay

Closing the issue, as we currently do not have SyPrimtives in the codebase

rasswanth-s avatar May 28 '23 11:05 rasswanth-s