Repository: OpenMined/PySyft
Pull request: fix#5365 skip rewrapping iter
## Description
Fix #5365

`__iter__` gets wrapped multiple times due to 
https://github.com/OpenMined/PySyft/blob/261869e50852a24b2d76f3b44a5819050acd9eb8/src/syft/ast/klass.py#L338-L342

Fix: skip re-wrapping if already wrapped

## Checklist
- [x] I have followed the [Contribution Guidelines](https://github.com/OpenMined/.github/blob/master/CONTRIBUTING.md) and [Code of Conduct](https://github.com/OpenMined/.github/blob/master/CODE_OF_CONDUCT.md)
- [x] I have commented my code following the [OpenMined Styleguide](https://github.com/OpenMined/.github/blob/master/STYLEGUIDE.md)
- [ ] I have labeled this PR with the relevant [Type labels](https://github.com/OpenMined/.github/labels?q=Type%3A)
- [x] My changes are covered by tests


Problem statement:
Improving Dict Iterator creation
## Description
Currently, if you trace it, we are making 4 remote `__len__` requests when creating a Dict/OrderedDict remote iterator. Ideally, we should stick to one. The same goes for any Iterator that is being created from Dict (`.values()`, `.items()`, etc).

Snippet to reproduce:
```
import syft as sy
client = sy.VirtualMachine().get_root_client()
iter(client.syft.lib.python.collections.OrderedDict({1: 1, 2: 2}))
```

with a `print(attr_path_and_name)` in `syft/ast/klass.py` at the top of the function `run_class_method`.

Hints from issue discussion:
This is caused due to the `__iter__` being wrapped multiple times by:

https://github.com/OpenMined/PySyft/blob/261869e50852a24b2d76f3b44a5819050acd9eb8/src/syft/ast/klass.py#L338-L342

For `OrderedDict`: (`items`, `values`, `keys`) return an `Iterator`, hence the  `__iter__` gets wrapped thrice.

I believe the wrapping was done in this manner to account for types that can't be iterated directly (no `__iter__`) but have inbuilt functions that can return an `Iterator`.

To avoid the wrapping multiple times, either a flag can be added within this `for` loop.
Or`wrap_iterator` function can check if `__iter__` has already been wrapped, and skip re-wrapping it (current PR).

@tudorcebere if this seems fine, I'll create a PR.

Commit message: str qualname fix

Patch to review:
diff --git a/src/syft/ast/klass.py b/src/syft/ast/klass.py
index b5fe1103120..79936093aef 100644
--- a/src/syft/ast/klass.py
+++ b/src/syft/ast/klass.py
@@ -232,6 +232,11 @@ def __iter__(self: Any) -> Iterable:
 
     attr_name = "__iter__"
     iter_target = attrs[attr_name]
+
+    # skip if __iter__ has already been wrapped
+    if "__qual_name__" in dir(iter_target) and "wrap_iter" in iter_target.__qualname__:
+        return
+
     if not callable(iter_target):
         traceback_and_raise(AttributeError("Can't wrap a non callable iter attribute"))
     else:


Code review comments:
diff_hunk: @@ -232,6 +232,11 @@ def __iter__(self: Any) -> Iterable:
 
     attr_name = "__iter__"
     iter_target = attrs[attr_name]
+
+    # skip if __iter__ has already been wrapped
+    if "__qual_name__" in dir(iter_target) and "wrap_iter" in iter_target.__qualname__:
path: src/syft/ast/klass.py
text: @user1:
NIT:
```
qual_name = getattr(iter_target, "__qual_name__", None)
if qual_name and "wrap_iter" in qual_name:
    return
```