Support localized-attributes settings #1060
Conversation
| @@ -2042,6 +2042,68 @@ def reset_proximity_precision(self) -> TaskInfo: | |||
|
|
|||
| return TaskInfo(**task) | |||
|
|
|||
| # LOCALIZED ATTRIBUTES SETTINGS | |||
|
|
|||
| def get_localized_attributes(self) -> Union[List[Dict[str, List[str]]], None]: | |||
There was a problem hiding this comment.
Not very experienced with python, let me know if I should have created a model for the localized attributes, similar to embedders or proximity precision.
Let's do that, it will make it easier to work with. So you will have:
class LocalizedAttributes(CamelBase):
...| def get_localized_attributes(self) -> Union[List[Dict[str, List[str]]], None]: | |
| def get_localized_attributes(self) -> Union[List[LocalizedAttributes], None]: |
There was a problem hiding this comment.
Thanks very much for the feedback, it's very detailed!
So by your suggestion the LocalizedAttributes class should encapsulate the list, which limits me to having a class with just one field for attributes.
Should I also make another class for each entry in the list?
I'm trying to find out if this should be similar to Rust structs, where you make struct after struct until you have the entire structure represented in code (sorry if you do not use rust, I suppose structs in C aren't too different in this respect)
P. S.: will fix this very soon
There was a problem hiding this comment.
Excellent question! You are correct this should have been a list. I updated my suggestion for this.
| MeilisearchApiError | ||
| An error containing details about why Meilisearch can't process your request. Meilisearch error codes are described here: https://www.meilisearch.com/docs/reference/errors/error_codes#meilisearch-errors | ||
| """ | ||
| return self.http.get(self.__settings_url_for(self.config.paths.localized_attributes)) |
There was a problem hiding this comment.
| return self.http.get(self.__settings_url_for(self.config.paths.localized_attributes)) | |
| attributes = self.http.get(self.__settings_url_for(self.config.paths.localized_attributes)) | |
| return [LocalizedAttributes(**attribute) for attribute in attributes] |
| def update_localized_attributes( | ||
| self, body: Union[List[Dict[str, List[str]]], None] | ||
| ) -> TaskInfo: |
There was a problem hiding this comment.
We have tried to move to Mapping for these to give more flexablitiy.
| def update_localized_attributes( | |
| self, body: Union[List[Dict[str, List[str]]], None] | |
| ) -> TaskInfo: | |
| def update_localized_attributes( | |
| self, body: Union[List[Mapping[str, List[str]]], None] | |
| ) -> TaskInfo: |
| @@ -0,0 +1,35 @@ | |||
| NEW_LOCALIZED_ATTRIBUTES = [{"attributePatterns": ["title"], "locales": ["eng"]}] | |||
There was a problem hiding this comment.
Update to use the new class.
| update = index.wait_for_task(response.task_uid) | ||
| assert update.status == "succeeded" | ||
| response = index.get_localized_attributes() | ||
| assert NEW_LOCALIZED_ATTRIBUTES == response |
There was a problem hiding this comment.
I believe this may fail after updating to a class, if it does changing to assert [x.model_dump for x in NEW_LOCALIZED_ATTRIBUTES] == [x.model_dump() for x in response] should fix it.
| assert update.status == "succeeded" | ||
| # Check the settings have been correctly updated | ||
| response = index.get_localized_attributes() | ||
| assert NEW_LOCALIZED_ATTRIBUTES == response |
There was a problem hiding this comment.
Same as previous, this may fail with class and the same fix should work if so.
You are correct here, it will work as is. |
Pull Request
Related issue
Fixes #1011
PR checklist
Please check if your PR fulfills the following requirements:
Not very experienced with python, let me know if I should have created a model for the localized attributes, similar to embedders or proximity precision.
Regarding #1011, I don't think anything needs to change in the search parameters, since those seem to be dynamic anyway.