Codecov Report

Base: 81.08% // Head: 81.67% // Increases project coverage by +0.59% 🎉

Coverage data is based on head (89222dc) compared to base (5fce6b6).
Patch coverage: 98.30% of modified lines in pull request are covered.

@@            Coverage Diff             @@
##             main     #808      +/-   ##
==========================================
+ Coverage   81.08%   81.67%   +0.59%     
==========================================
  Files         138      138              
  Lines        7067     7112      +45     
==========================================
+ Hits         5730     5809      +79     
+ Misses       1337     1303      -34     

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

We now face some limitations:

1. Bool flag root_id is added to each storage backend config with default value True. But in-memory it is always True because it doesn't have a config or flag.
2. With root_id=True by default, users may receive many warnings when they want to add or change Documents on nested level directly without setting _root_id_ or parent_id manually. Like da['@c'].extend(Document())
3. If we remove the root_id flag and always store this infomation by default, and only raise warning or error when user do da.find(query, on='@c', return_root=True), user may need to re-index the database.

We now face some limitations:

1. Bool flag `root_id` is added to each storage backend config with default value `True`. But `in-memory` it is always True because it doesn't have a config or flag.

2. With `root_id=True` by default, users may receive many warnings when they want to add or change Documents on nested level directly without setting `_root_id_` or `parent_id` manually. Like `da['@c'].extend(Document())`

3. If we remove the `root_id` flag and always store this infomation by default, and only raise warning or error when user do `da.find(query, on='@c', return_root=True)`, user may need to re-index the database.

To add a little bit more context to 2. and 3.:

The intended use of this feature is for someone to add Documents to the root-level DocumentArray, so that Documents in a subindex can automatically be 'tagged' with a root_id: da.extend(...).
But if a user modifies a subindex dicrectly, we can't know the roots of the documents that they add (there might not even be a root): da['@c'].extend(...). So, we cannot support this pattern.

The question becomes, when do we warn about this behaviour? At insert time, or at find() time when the missing root_id actually starts to matter? That is what 2. and 3. above refer to.

We now face some limitations:

1. Bool flag `root_id` is added to each storage backend config with default value `True`. But `in-memory` it is always True because it doesn't have a config or flag.

2. With `root_id=True` by default, users may receive many warnings when they want to add or change Documents on nested level directly without setting `_root_id_` or `parent_id` manually. Like `da['@c'].extend(Document())`

3. If we remove the `root_id` flag and always store this infomation by default, and only raise warning or error when user do `da.find(query, on='@c', return_root=True)`, user may need to re-index the database.

To add a little bit more context to 2. and 3.:

The intended use of this feature is for someone to add Documents to the root-level DocumentArray, so that Documents in a subindex can automatically be 'tagged' with a root_id: da.extend(...). But if a user modifies a subindex dicrectly, we can't know the roots of the documents that they add (there might not even be a root): da['@c'].extend(...). So, we cannot support this pattern.

The question becomes, when do we warn about this behaviour? At insert time, or at find() time when the missing root_id actually starts to matter? That is what 2. and 3. above refer to.

If u have root_id True and you are extending in the wrong way, add the warning, to me this makes sense. And warn the earliest the better

📝 Docs are deployed on https://ft-feat-root-id--jina-docs.netlify.app 🎉