Add language detection to REST API #659

UnniKohonen · 2022-12-29T12:42:07Z

This PR adds the ability to detect the language of a text to the REST API. The language detection uses the simplemma python library.

A POST method is added to the end-point /detect-language. It expects the request body to include a json object with the text whose language is to be detected and a list of candidate languages as their ISO 639-1 codes. For example:

{
  "candidates": ["en", "fi"],
  "text": "A quick brown fox jumped over the lazy dog."
}

The response is a json object with the format:

{
  "results": [
    {"language": "en", "score": 0.85},
    {"language": "fi", "score": 0.1},
    {"language": null, "score": 0.1} 
  ]
}

where the scores range from 0 to 1 and a null value is used for an unknown language.

Implements #631

codecov · 2022-12-29T12:46:03Z

Codecov Report

Base: 99.55% // Head: 99.53% // Decreases project coverage by -0.02% ⚠️

Coverage data is based on head (34c2538) compared to base (45be34d).
Patch coverage: 100.00% of modified lines in pull request are covered.

Additional details and impacted files

@@            Coverage Diff             @@
##           master     #659      +/-   ##
==========================================
- Coverage   99.55%   99.53%   -0.02%     
==========================================
  Files          87       87              
  Lines        6006     6047      +41     
==========================================
+ Hits         5979     6019      +40     
- Misses         27       28       +1

Impacted Files	Coverage Δ
annif/rest.py	`97.05% <100.00%> (+0.69%)`	⬆️
tests/test_rest.py	`100.00% <100.00%> (ø)`
annif/backend/omikuji.py	`96.15% <0.00%> (-1.29%)`	⬇️
tests/test_config.py	`100.00% <0.00%> (ø)`

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

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

juhoinkinen · 2023-01-02T10:47:44Z

I see this is only a draft at the moment, but I took a glance and I think it would be better that the end-point name had hyphen instead of underscore (/detect_language - > /detect-language), it seems to be the preferred convention.

osma

Looks like a good start. I gave some comments on individual code lines. In addition to those:

black formatting should be applied (see details)
there should be a unit test in tests/test_rest.py which exercises the detect_language method

osma · 2023-01-09T14:35:16Z

annif/openapi/annif.yaml

+  /detect-language:
+    post:
+      tags:
+      - Languages detection


I think this should be "Language detection"

osma · 2023-01-09T14:45:37Z

annif/openapi/annif.yaml

+                  example: A quick brown fox jumped over the lazy dog.
+                candidates:
+                  type: array
+                  description: candidate languages as ISO 639-1 codes


Please change "ISO 639-1 codes" to "IETF BCP 47 codes", because not all supported languages are necessarily in ISO 639-1. (I noticed the same problem in Simplemma README and opened an issue there)

osma · 2023-01-09T14:46:27Z

annif/openapi/annif.yaml

+                  description: candidate languages as ISO 639-1 codes
+                  items:
+                    type: string
+                    maxLength: 2


Should be 3, as some valid BCP 47 language tags (e.g. enm, hbs) may have 3 characters

osma · 2023-01-09T14:50:07Z

annif/rest.py

+    scores = lang_detector(body.get("text"), tuple(body.get("candidates")))
+    return {
+        "results": [
+            {"language": s[0] if s[0] != "unk" else None, "score": s[1]}


How about this instead:

{"language": lang if lang != "unk" else None, "score": score} for lang, score in scores

I think it would be easier to understand than the numeric indexing.

osma · 2023-01-16T08:26:38Z

Thanks for adding tests. A few more things:

There should be tests for special cases, e.g. empty input, no candidate languages, unknown or malformed candidate languages...
What if there are 20 candidate languages? How much memory will it take to handle the query? Will the memory be released afterwards? (I think not, Simplemma keeps models in memory AFAIK)

UnniKohonen · 2023-01-17T14:03:09Z

Right now, when making a request with no candidates or unknown candidates, the endpoint returns an empty list and when making a request with no text, it returns { "language": null, "score": 1 }. Does this make sense? Or should it always return the unknown language with score 1 when the input is incorrect?

I also tested making a request with all 48 possible language candidates. I had about 4 GB of free memory which was used amost completely after making the request. The memory isn't released automatically afterwards but it is freed if the endpoint is accessed again (simplemma is run again) or annif is restarted. Making other requests also slows down a lot after runnig simplemma with all candidates.

osma · 2023-01-18T08:38:18Z

Right now, when making a request with no candidates or unknown candidates, the endpoint returns an empty list

The good news is that it's not crashing! 😁

My opinions on these cases:

With no candidates given, I think it would be good to return a 400 Bad Request status code, with a descriptive error message.
With unknown candidates (language codes that are unrecognized/unsupported by simplemma), I think a 400 Bad Request with a descriptive error message would also be appropriate.

There should be unit tests to check that these are indeed the results.

and when making a request with no text, it returns { "language": null, "score": 1 }. Does this make sense? Or should it always return the unknown language with score 1 when the input is incorrect?

I think this is OK, but there should also be a unit test for this special case.

I also tested making a request with all 48 possible language candidates. I had about 4 GB of free memory which was used amost completely after making the request. The memory isn't released automatically afterwards but it is freed if the endpoint is accessed again (simplemma is run again) or annif is restarted. Making other requests also slows down a lot after runnig simplemma with all candidates.

Great, thanks for testing! This is mostly what I suspected, although it's a surprise that accessing the endpoint again will free the memory. (Maybe this has to do with Flask running in development mode?)

This has some potential for DoS situations (intended or not), but I guess it's hard for us to avoid that given how Simplemma works. We could, however, limit the number of candidate languages per request to, say, at most 5. What do others think? @juhoinkinen ?

We could also try to work with the Simplemma maintainer if we want to change the way Simplemma allocates and releases models. For example, it could be possible to ask Simplemma to release the memory immediately or after a set period like 60 seconds after use.

juhoinkinen · 2023-01-18T09:25:13Z

I also tested making a request with all 48 possible language candidates. I had about 4 GB of free memory which was used amost completely after making the request. The memory isn't released automatically afterwards but it is freed if the endpoint is accessed again (simplemma is run again) or annif is restarted. Making other requests also slows down a lot after runnig simplemma with all candidates.

Great, thanks for testing! This is mostly what I suspected, although it's a surprise that accessing the endpoint again will free the memory. (Maybe this has to do with Flask running in development mode?)

This has some potential for DoS situations (intended or not), but I guess it's hard for us to avoid that given how Simplemma works. We could, however, limit the number of candidate languages per request to, say, at most 5. What do others think? @juhoinkinen ?

Limiting the number of candidate languages seems reasonable. If there is no simple way to make the limit configurable, 5 could be a good number for that.

We could also try to work with the Simplemma maintainer if we want to change the way Simplemma allocates and releases models. For example, it could be possible to ask Simplemma to release the memory immediately or after a set period like 60 seconds after use.

I noticed there is an issue in Simplemma repository about loading models to memory, which was opened just yesterday.

sonarcloud · 2023-01-19T12:09:29Z

Kudos, SonarCloud Quality Gate passed!

0 Bugs
0 Vulnerabilities
0 Security Hotspots
2 Code Smells

No Coverage information
0.0% Duplication

juhoinkinen · 2023-01-19T12:49:02Z

Just started to think, if some some testing could be performed also in tests/test_swagger.py. I don't remember just what more functionality does the tests in test_swagger.py cover than those in test_rest.py (if any).

juhoinkinen · 2023-01-19T14:24:22Z

Just started to think, if some some testing could be performed also in tests/test_swagger.py. I don't remember just what more functionality does the tests in test_swagger.py cover than those in test_rest.py (if any).

Background to the question of test_swagger.py vs test_rest.py: #551 (comment)

UnniKohonen added 2 commits December 29, 2022 14:01

Add language detection to REST API

4369a6c

Use a json object in request body

8f07fab

UnniKohonen added the enhancement label Dec 29, 2022

Change endpoint name

e3a355f

osma requested changes Jan 9, 2023

View reviewed changes

UnniKohonen added 3 commits January 10, 2023 12:34

Add unit tests

62b502f

Fix OpenAPI spec

2912177

Fix for loop

0160268

UnniKohonen added 2 commits January 19, 2023 14:07

Add error status codes

727236c

Add unit tests for bad requests

34c2538

osma mentioned this pull request Jan 19, 2023

Refactor/better caching strategy adbar/simplemma#34

Closed

juhoinkinen linked an issue May 11, 2023 that may be closed by this pull request

Language detection method in REST API & CLI #631

Open

osma mentioned this pull request Aug 9, 2023

Plans for simplemma 1.0 release? adbar/simplemma#110

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add language detection to REST API #659

Add language detection to REST API #659

UnniKohonen commented Dec 29, 2022 •

edited

codecov bot commented Dec 29, 2022 •

edited

juhoinkinen commented Jan 2, 2023

osma left a comment

osma Jan 9, 2023

osma Jan 9, 2023

osma Jan 9, 2023

osma Jan 9, 2023

osma commented Jan 16, 2023

UnniKohonen commented Jan 17, 2023

osma commented Jan 18, 2023

juhoinkinen commented Jan 18, 2023 •

edited

sonarcloud bot commented Jan 19, 2023

juhoinkinen commented Jan 19, 2023

juhoinkinen commented Jan 19, 2023

Add language detection to REST API #659

Are you sure you want to change the base?

Add language detection to REST API #659

Conversation

UnniKohonen commented Dec 29, 2022 • edited

codecov bot commented Dec 29, 2022 • edited

Codecov Report

juhoinkinen commented Jan 2, 2023

osma left a comment

Choose a reason for hiding this comment

osma Jan 9, 2023

Choose a reason for hiding this comment

osma Jan 9, 2023

Choose a reason for hiding this comment

osma Jan 9, 2023

Choose a reason for hiding this comment

osma Jan 9, 2023

Choose a reason for hiding this comment

osma commented Jan 16, 2023

UnniKohonen commented Jan 17, 2023

osma commented Jan 18, 2023

juhoinkinen commented Jan 18, 2023 • edited

sonarcloud bot commented Jan 19, 2023

juhoinkinen commented Jan 19, 2023

juhoinkinen commented Jan 19, 2023

UnniKohonen commented Dec 29, 2022 •

edited

codecov bot commented Dec 29, 2022 •

edited

juhoinkinen commented Jan 18, 2023 •

edited