Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: move vega charts to API #185

Merged
merged 7 commits into from
Jun 25, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 22 additions & 2 deletions app/_types.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@

from . import config
from .utils import str_utils
from .validations import check_all_facets_fields_are_agg, check_index_id_is_defined
from .validations import check_all_values_are_fields_agg, check_index_id_is_defined

#: A precise expectation of what mappings looks like in json.
#: (dict where keys are always of type `str`).
Expand Down Expand Up @@ -42,6 +42,9 @@ class FacetInfo(BaseModel):
FacetsInfos = dict[str, FacetInfo]
"""Information about facets for a search result"""

ChartsInfos = dict[str, JSONType]
"""Information about charts for a search result"""

FacetsFilters = dict[str, list[str]]
"""Data about selected filters for each facet: facet name -> list of values"""

Expand All @@ -67,6 +70,7 @@ class SuccessSearchResponse(BaseModel):
hits: list[JSONType]
aggregations: JSONType | None = None
facets: FacetsInfos | None = None
charts: ChartsInfos | None = None
page: int
page_size: int
page_count: int
Expand Down Expand Up @@ -206,6 +210,13 @@ class SearchParameters(BaseModel):
If None (default) no facets are returned."""
),
] = None
charts: Annotated[
list[str] | None,
Query(
description="""Name of vega representations to return in the response.
If None (default) no charts are returned."""
),
] = None
sort_params: Annotated[
JSONType | None,
Query(
Expand Down Expand Up @@ -304,7 +315,15 @@ def sort_by_scripts_needs_params(self):
@model_validator(mode="after")
def check_facets_are_valid(self):
"""Check that the facets names are valid."""
errors = check_all_facets_fields_are_agg(self.index_id, self.facets)
errors = check_all_values_are_fields_agg(self.index_id, self.facets)
if errors:
raise ValueError(errors)
return self

@model_validator(mode="after")
def check_charts_are_valid(self):
"""Check that the graph names are valid."""
errors = check_all_values_are_fields_agg(self.index_id, self.charts)
if errors:
raise ValueError(errors)
return self
Expand Down Expand Up @@ -353,4 +372,5 @@ class GetSearchParamsTypes:
fields = _annotation_new_type(str, SEARCH_PARAMS_ANN["fields"])
sort_by = SEARCH_PARAMS_ANN["sort_by"]
facets = _annotation_new_type(str, SEARCH_PARAMS_ANN["facets"])
charts = _annotation_new_type(str, SEARCH_PARAMS_ANN["charts"])
index_id = SEARCH_PARAMS_ANN["index_id"]
3 changes: 3 additions & 0 deletions app/api.py
Original file line number Diff line number Diff line change
Expand Up @@ -107,12 +107,14 @@ def search_get(
fields: GetSearchParamsTypes.fields = None,
sort_by: GetSearchParamsTypes.sort_by = None,
facets: GetSearchParamsTypes.facets = None,
charts: GetSearchParamsTypes.charts = None,
index_id: GetSearchParamsTypes.index_id = None,
) -> SearchResponse:
# str to lists
langs_list = langs.split(",") if langs else ["en"]
fields_list = fields.split(",") if fields else None
facets_list = facets.split(",") if facets else None
charts_list = charts.split(",") if charts else None
# create SearchParameters object
try:
search_parameters = SearchParameters(
Expand All @@ -123,6 +125,7 @@ def search_get(
fields=fields_list,
sort_by=sort_by,
facets=facets_list,
charts=charts_list,
index_id=index_id,
)
return app_search.search(search_parameters)
Expand Down
122 changes: 122 additions & 0 deletions app/charts.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,122 @@
from ._types import ChartsInfos, SuccessSearchResponse


def build_charts(
search_result: SuccessSearchResponse,
charts_names: list[str] | None,
) -> ChartsInfos:
charts: ChartsInfos = {}
aggregations = search_result.aggregations

if charts_names is None or aggregations is None:
return charts

for chart_name in charts_names:
agg_data = aggregations.get(chart_name, {})

buckets = agg_data.get("buckets", []) if agg_data else []

# Filter unknown values
values = [
{"category": bucket["key"], "amount": bucket["doc_count"]}
for bucket in buckets
if bucket["key"] != "unknown"
]
values.sort(key=lambda x: x["category"])

charts[chart_name] = {
"$schema": "https://vega.github.io/schema/vega/v5.json",
"title": chart_name,
"autosize": {"type": "fit", "contains": "padding"},
"signals": [
{
"name": "width",
"init": "containerSize()[0]",
"on": [{"events": "window:resize", "update": "containerSize()[0]"}],
},
{
"name": "tooltip",
"value": {},
"on": [
{"events": "rect:pointerover", "update": "datum"},
{"events": "rect:pointerout", "update": "{}"},
],
},
],
"height": 140,
"padding": 5,
"data": [
{
"name": "table",
"values": values,
},
],
"scales": [
{
"name": "xscale",
"type": "band",
"domain": {"data": "table", "field": "category"},
"range": "width",
"padding": 0.05,
"round": True,
},
{
"name": "yscale",
"domain": {"data": "table", "field": "amount"},
"nice": True,
"range": "height",
},
],
"axes": [
{"orient": "bottom", "scale": "xscale", "domain": False, "ticks": False}
],
"marks": [
{
"type": "rect",
"from": {"data": "table"},
"encode": {
"enter": {
"x": {"scale": "xscale", "field": "category"},
"width": {"scale": "xscale", "band": 1},
"y": {"scale": "yscale", "field": "amount"},
"y2": {"scale": "yscale", "value": 0},
},
"update": {
"fill": {"value": "steelblue"},
},
"hover": {
"fill": {"value": "red"},
},
},
},
{
"type": "text",
"encode": {
"enter": {
"align": {"value": "center"},
"baseline": {"value": "bottom"},
"fill": {"value": "#333"},
},
"update": {
"x": {
"scale": "xscale",
"signal": "tooltip.category",
"band": 0.5,
},
"y": {
"scale": "yscale",
"signal": "tooltip.amount",
"offset": -2,
},
"text": {"signal": "tooltip.amount"},
"fillOpacity": [
{"test": "datum === tooltip", "value": 0},
{"value": 1},
],
},
},
},
],
}
Comment on lines +14 to +120
Copy link
Collaborator

@Kout95 Kout95 Jun 25, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For me, you have to give data by API and then build the options in frontend like this :
This allows to avoid chart options for every graph

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

After discussion, we will keep it like this right now. Might be re-discussed with Alex.


return charts
11 changes: 7 additions & 4 deletions app/query.py
Original file line number Diff line number Diff line change
Expand Up @@ -281,15 +281,15 @@ def parse_sort_by_script(


def create_aggregation_clauses(
config: IndexConfig, facets: list[str] | None
config: IndexConfig, fields: set[str] | list[str] | None
) -> dict[str, Agg]:
"""Create term bucket aggregation clauses
for all fields corresponding to facets,
as defined in the config
"""
clauses = {}
if facets is not None:
for field_name in facets:
if fields is not None:
for field_name in fields:
field = config.fields[field_name]
if field.bucket_agg:
# TODO - aggregation might depend on agg type or field type
Expand Down Expand Up @@ -337,7 +337,10 @@ def build_es_query(
if q.filter_query:
es_query = es_query.query("bool", filter=q.filter_query)

for agg_name, agg in create_aggregation_clauses(config, params.facets).items():
agg_fields = set(params.facets) if params.facets is not None else set()
if params.charts is not None:
agg_fields.update(params.charts)
for agg_name, agg in create_aggregation_clauses(config, agg_fields).items():
es_query.aggs.bucket(agg_name, agg)

sort_by: JSONType | str | None = None
Expand Down
5 changes: 4 additions & 1 deletion app/search.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@

from . import config
from ._types import SearchParameters, SearchResponse, SuccessSearchResponse
from .charts import build_charts
from .facets import build_facets
from .postprocessing import BaseResultProcessor, load_result_processor
from .query import build_elasticsearch_query_builder, build_search_query, execute_query
Expand Down Expand Up @@ -37,13 +38,14 @@ def search(
)
logger.debug(
"Received search query: q='%s', langs='%s', page=%d, "
"page_size=%d, fields='%s', sort_by='%s'",
"page_size=%d, fields='%s', sort_by='%s', charts='%s'",
params.q,
params.langs_set,
params.page,
params.page_size,
params.fields,
params.sort_by,
params.charts,
)
index_config = params.index_config
query = build_search_query(
Expand All @@ -69,6 +71,7 @@ def search(
search_result.facets = build_facets(
search_result, query, params.main_lang, index_config, params.facets
)
search_result.charts = build_charts(search_result, params.charts)
# remove aggregations to avoid sending too much information
search_result.aggregations = None
return search_result
13 changes: 7 additions & 6 deletions app/validations.py
Original file line number Diff line number Diff line change
Expand Up @@ -22,19 +22,20 @@ def check_index_id_is_defined(index_id: str | None, config: Config) -> None:
)


def check_all_facets_fields_are_agg(
index_id: str | None, facets: list[str] | None
def check_all_values_are_fields_agg(
index_id: str | None, values: list[str] | None
) -> list[str]:
"""Check all facets are valid,
that is, correspond to a field with aggregation"""
"""Check that all values are fields with aggregate: true
property, that is, correspond to a field with aggregation.
Used to check that charts are facets are valid."""
errors: list[str] = []
if facets is None:
if values is None:
return errors
global_config = cast(Config, CONFIG)
index_id, index_config = global_config.get_index_config(index_id)
if index_config is None:
raise ValueError(f"Cannot get index config for index_id {index_id}")
for field_name in facets:
for field_name in values:
if field_name not in index_config.fields:
errors.append(f"Unknown field name in facets: {field_name}")
elif not index_config.fields[field_name].bucket_agg:
Expand Down
6 changes: 3 additions & 3 deletions frontend/public/off.html
Original file line number Diff line number Diff line change
Expand Up @@ -417,9 +417,9 @@
</div>
</div>
<div class="large-2 columns">
<searchalicious-chart search-name="off" label="Nutri-score distribution" key="nutriscore_grade", categories='["a", "b", "c", "d", "e", "unknown", "not-applicable"]''></searchalicious-chart>
<searchalicious-chart search-name="off" label="Nova score distribution" key="nova_group", categories='["1", "2", "3", "4", "undefined", "not-applicable"]'></searchalicious-chart>
<searchalicious-chart search-name="off" label="Eco score distribution" key="ecoscore_grade", categories='["a", "b", "c", "d", "e", "unknown", "not-applicable"]''></searchalicious-chart>
<searchalicious-chart search-name="off" name="nutrition_grades"'></searchalicious-chart>
<searchalicious-chart search-name="off" name="ecoscore_grade"></searchalicious-chart>
<searchalicious-chart search-name="off" name="nova_groups"></searchalicious-chart>
</div>
</div>
</div>
Expand Down
1 change: 1 addition & 0 deletions frontend/src/events.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ export interface SearchResultDetail extends BaseSearchDetail {
pageSize: number;
currentPage: number;
facets: Object; // FIXME: we could be more precise
charts: Object; // FIXME: we could be more precise
}

/**
Expand Down
26 changes: 26 additions & 0 deletions frontend/src/mixins/search-ctl.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import {
SearchaliciousHistoryInterface,
SearchaliciousHistoryMixin,
} from './history';
import {SearchaliciousChart} from '../search-chart';

export interface SearchParameters extends SortParameters {
q: string;
Expand All @@ -30,6 +31,8 @@ export interface SearchParameters extends SortParameters {
page?: string;
index_id?: string;
facets?: string[];
params?: string[];
charts?: string[];
}
export interface SearchaliciousSearchInterface
extends EventRegistrationInterface,
Expand Down Expand Up @@ -137,6 +140,17 @@ export const SearchaliciousSearchMixin = <T extends Constructor<LitElement>>(
);
}

/**
* Return the list of searchalicious-chart nodes
*/
_chartsNodes(): SearchaliciousChart[] {
return Array.from(
document.querySelectorAll(
`searchalicious-chart[search-name=${this.name}`
)
);
}

/**
* Select a term by taxonomy in all facets
* It will update the selected terms in facets
Expand Down Expand Up @@ -228,6 +242,14 @@ export const SearchaliciousSearchMixin = <T extends Constructor<LitElement>>(
return [...new Set(names)];
}

/**
* Get the list of charts we want to request
*/
_charts(): string[] {
const names = this._chartsNodes().map((chart) => chart.getName());
return [...new Set(names)];
}

/**
* Get the filter linked to facets
* @returns an expression to be added to query
Expand Down Expand Up @@ -380,6 +402,9 @@ export const SearchaliciousSearchMixin = <T extends Constructor<LitElement>>(
if (this._facets().length > 0) {
params.facets = this._facets();
}
if (this._charts().length > 0) {
params.charts = this._charts();
}
return params;
};

Expand Down Expand Up @@ -419,6 +444,7 @@ export const SearchaliciousSearchMixin = <T extends Constructor<LitElement>>(
currentPage: this._currentPage!,
pageSize: this.pageSize,
facets: data.facets,
charts: data.charts,
};
this.dispatchEvent(
new CustomEvent(SearchaliciousEvents.NEW_RESULT, {
Expand Down
Loading