Skip to content
This repository was archived by the owner on Sep 9, 2026. It is now read-only.
This repository was archived by the owner on Sep 9, 2026. It is now read-only.

Extending DocList with itself causes unexpected recursion #1489

Closed

Description

@movchan74

Description

When extending a DocList with itself, the code seems to add the list recursively, causing a huge increase in the length of the list. For example, the following code produces a DocList of length 9,052,000 (I had to interrupt the execution):

from docarray import DocVec, BaseDoc, DocList
from docarray.documents.image import ImageDoc
docs = DocList[ImageDoc]([ImageDoc(url=f'http://url.com/{i}.png') for i in range(10)])
docs.extend(docs)

However, a similar operation works fine with Python lists:

l = list(range(10))
l.extend(l)
print(l)  # [0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 0, 1, 2, 3, 4, 5, 6, 7, 8, 9]

Questions

  • Why does DocList behave differently than Python lists in this situation?
  • Are there plans to support the extend method for DocVec in the future?

Steps to reproduce

  1. Run the following code snippet:
from docarray import DocVec, BaseDoc, DocList
from docarray.documents.image import ImageDoc
docs = DocList[ImageDoc]([ImageDoc(url=f'http://url.com/{i}.png') for i in range(10)])
docs.extend(docs)
  1. Interrupt the execution because it hangs.

  2. Observe that the resulting DocList has an enourmous length, rather than the expected 20.

Activity

  1. moved this to Todo/Bug in DocArray backlogon May 4, 2023
  2. JoanFM commented on May 4, 2023

    @JoanFM
    Member

    Hey @movchan74 ,

    Thanks for the very helpful reports. We recently merged a PR #1457, which I suspect may fix the issue. Maybe u can try on the main branch? otherwise I will try tomorrow. In case it does solve it, this will be released very soon.

    As per plans for supporting extend of DocVec, there is no explicit plan for it, but if community asks we would be happy to consider adding it.

  3. movchan74 commented on May 4, 2023

    @movchan74
    Author

    I've tried it on the main branch but the issue is still present.

    Regarding the extend for DocVec, it would be really nice to have the same behavior for DocVec and DocList.

  4. JoanFM commented on May 5, 2023

    @JoanFM
    Member

    Comment on my side:

    This is the error we are making in the code, with an equivalent code and result:

    class MyList(list):
        def extend(self, __iterable) -> None:
            def generator(aux):
                for a in aux:
                    yield a
    
            return super().extend(generator(__iterable))
    
        def append(self, __object) -> None:
            return super().append(__object)
    
    
    l = MyList([1, 2, 3, 4])
    
    l.extend(l)
    
  5. linked a pull request that will close this issuefix: fix extend with itself infinite recursion #1493on May 5, 2023
  6. moved this from Todo/Bug to In Progress in DocArray backlogon May 5, 2023
  7. JoanFM commented on May 5, 2023

    @JoanFM
    Member

    Hey @movchan74 ,

    Let me try to answer the 2 questions:

    • Why does it behave differently? Because we do some validations and we iterate over the Documents in the container as a generator, which makes it infinite since it keeps adding to the list. This will be fixed soon.

    • As if we want to add extend to DocVec. We are not so sure about this, DocVec is much more planned to be static in terms of size, and will be complex to provide a coherent list-like behavior. In past versions we tried to fit every pattern in a single type of object, and we now decided to clarify the interfaces. We believe it is better to do DocVec.to_doc_list, do the extension and then convert back. However, we welcome you to join our discord channel to discuss this or any other design choices.

  8. self-assigned this
    on May 5, 2023
  9. samsja commented on May 5, 2023

    @samsja
    Member

    Are there plans to support the extend method for DocVec in the future?

    Here we want to be cautious about how we integrate an extended method on DocVec because extending on a DocVec will be slower that on a DocList (because of underlying implementation)

    The way I see it we could either have:

    1- extend as a method of DocVec that add the new DocVec to self
    2- cat function like in torch/numpy that will fuse to DocVec into one

    1 is more coherent with DocList. 2 is more intuitive and gives more insight into what is happening under the hood.

    @movchan74 any opinion on what you would prefer ?

  10. movchan74 commented on May 5, 2023

    @movchan74
    Author

    I think extend makes more sense to keep it consistent with DocList.

  11. JoanFM commented on May 5, 2023

    @JoanFM
    Member

    @movchan74 ,

    Our concern is that we wanted in purpose not to have consistency with DocList so that it is clear that they are different things and behave differently.

    We have the impression that adding appends and extends support people will expect list-like behavior which we cannot ensure

  12. JoanFM commented on May 5, 2023

    @JoanFM
    Member

    BTW; I created this discussion #1495

    So that this valuable discussion is not hidden in a not-so-related bug report

  13. moved this from In Progress to Done in DocArray backlogon May 8, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Labels

No labels
No labels

Type

No type

Projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions