You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
This repository was archived by the owner on Sep 9, 2026. It is now read-only.
Repository navigation
This repository was archived by the owner on Sep 9, 2026. It is now read-only.
Extending DocList with itself causes unexpected recursion #1489
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):
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.
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.
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.
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
Activity
JoanFM commentedon May 4, 2023
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.
movchan74 commentedon May 4, 2023
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.
JoanFM commentedon May 5, 2023
Comment on my side:
This is the error we are making in the code, with an equivalent code and result:
JoanFM commentedon May 5, 2023
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
extendto 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 coherentlist-likebehavior. 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 doDocVec.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.samsja commentedon May 5, 2023
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-
extendas a method of DocVec that add the new DocVec to self2-
catfunction like in torch/numpy that will fuse to DocVec into one1 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 ?
movchan74 commentedon May 5, 2023
I think
extendmakes more sense to keep it consistent with DocList.JoanFM commentedon May 5, 2023
@movchan74 ,
Our concern is that we wanted in purpose not to have consistency with
DocListso that it is clear that they are different things and behave differently.We have the impression that adding
appendsandextendssupport people will expectlist-likebehavior which we cannot ensureJoanFM commentedon May 5, 2023
BTW; I created this discussion #1495
So that this valuable discussion is not hidden in a not-so-related bug report