Implement Ancestry Layer-wise feature API #605
Conversation
|
We need to have proper test infrastructure. The previous code doesn't work at all but passes all test checks. Now it's fixed though. |
database/models.go
Outdated
| Ancestry | ||
|
|
||
| ProcessedBy Processors | ||
| Features []NamespacedFeature | ||
| // TODO(sidchen) deduplicate the Layers. |
There was a problem hiding this comment.
is this something we want to do? won't the different layers have different context?
There was a problem hiding this comment.
Well, the problem here is that Ancestry has Layers field, which contains a list of Layer, and AncestryLayer also has Layer. The information is duplicated. I just don't really know a good way to deduplicate these.
There was a problem hiding this comment.
Why not have both types be the same but leave the DetectedFeatures for each Layer empty when it isn't AncestryWithContent?
There was a problem hiding this comment.
I remember the design is that: Every struct, when returned, MUST have all fields filled to represent the whole information. Therefore, leaving DetectedFeatures empty will indicate that the Layer has NO detected features.
database/pgsql/ancestry.go
Outdated
| } | ||
|
|
||
| return awf, true, nil | ||
| // By the assumption of Ancestry Layer Index, we have the ancestry's layer | ||
| // index corresponding to the index in the array. |
There was a problem hiding this comment.
This should really be in the Session interface doc comment for FindAncestryWithContent.
There was a problem hiding this comment.
Well, this is an implementation detail for the ancestry_layer table. Here, I'm using the ancestry_index as the key to associate features to ancestry layers.
For people who implements this interface, they don't need to know what ancestry_index is. All they should do is get corresponding features for each layer in the Ancestry. They can do it in different ways, for example, query layer-wise feature directly using a single query.
KeyboardNerd
force-pushed
the
sidchen/feature
branch
from
01ffcf5 to
52bf0e4
Compare
database/models.go
Outdated
| ProcessedBy Processors | ||
| Features []NamespacedFeature | ||
| // TODO(sidchen) deduplicate the Layers. | ||
| Layers []AncestryLayer |
There was a problem hiding this comment.
We should comment here about the order of the layers.
Feature extraction algorithm is changed to associate features with ancestry layer. Database is updated to keep the relationship.
KeyboardNerd
force-pushed
the
sidchen/feature
branch
from
347890f to
2827b93
Compare
Ancestry now has the per-layer feature information instead of per-ancestry feature information.