Skip to content

Conversation

@bugsyb
Copy link
Contributor

@bugsyb bugsyb commented Apr 18, 2025

Initial code returned original string, whilst from logic perspective, at least for me, if there's no match/correction, it should return None.

p.s. thanks for prepping this interface to LanguateTool - saved me time. :)

@mdevolde
Copy link
Collaborator

Maybe it's more idiomatic to keep the return types consistent and keep to strings only?

@bugsyb
Copy link
Contributor Author

bugsyb commented Apr 24, 2025

There is another issue with previous/current logic, including this PR.
In case of correct word, it also does return 'None' atm. It's due to:

matches = [match for match in matches if match.replacements]

Filtering on match.replacements which is empty in case of correct string.

Current way of approach is to signal on return:

  1. No suggestion (no match.replacements), no close match (checked word is gibberish or anything).
  • initial matches return from response lists a "rule" which is believed to be failed.
  1. Word submit is correct (no match.replacements) too.
  • returns completely empty 'matches' at LanguageTool server response level.

I'd be keen to hear suggestion to return None for 1 or 2? Equally for the second situation idea is to return '' (empty string).

Any thoughts on this?
To keep as much as possible compatibility with current code utilizing language_tool_python.

Local mod at my end is to return:

  • None for exact match, as LanguageTool does not respond any matches.
  • 'str()'

bugsyb added 3 commits April 24, 2025 20:35
…est and str() for lack of suggestion, i.e. gibberish submit
… case of correct word submit for test and str() for lack of suggestion, i.e. gibberish submit
@mdevolde
Copy link
Collaborator

mdevolde commented Apr 25, 2025

I still don't think it's desirable to have the behavior you've implemented.
Firstly, it's not idiomatic, but even in the usual case of using the correct func, where you just want the corrected text, it seems normal to me, even if the text is already correct, to get it back, as in this example :

import language_tool_python

with language_tool_python.LanguageTool('en-US') as tool:
    corrected tool.correct('This is a cat.')

print(corrected)

Here I'm processing a literal, but in real use it's to process user input or something like this, so we don't know if the text is correct at the beginning, but even if it is, our aim is to get it back correct, so if it already is, why not return it?

(And furthermore, it's not backwards compatible.)

@bugsyb
Copy link
Contributor Author

bugsyb commented Apr 27, 2025

How would you distinguish with current code that submit: "Thisdasdasebadtefword" (not tested, just an example of gibberish), that correct function returns?
Currently there is no way to identify when no match is found or the submit text is correct. The latter could be worked around externally by comparing submit text to returned text.

Please suggest a working solution for use cases as explained above (correct text submit, correctable text submit, complete gibberish submit) - I might be looking at it from a wrong angle maybe because of the use case. Thanks.

@mdevolde
Copy link
Collaborator

I made a pull request on your repo! :)

@mdevolde
Copy link
Collaborator

mdevolde commented May 6, 2025

@bugsyb Don't forget to review on your fork so that PR can move forward. Thanks !

Amazing, thank you very much!
@mdevolde
Copy link
Collaborator

The requested change is implemented, without breaking change.
@jxmorris12 I can add documentation about this in the README if necessary. And the version should probably be bumped if the PR is accepted ;)

@jxmorris12
Copy link
Owner

@mdevolde Thanks, as always! And ok, if you don't mind adding documentation and bumping the version I can get a new version out by tomorrow.

@mdevolde
Copy link
Collaborator

@jxmorris12 This isn't my pull request and I'm not a maintainer, so I can't add a commit to this branch, but if you merge this PR, I can create a new one with the version bump and the doc ;))

@mdevolde
Copy link
Collaborator

By the way, it might be nice to rebase (squash in first commit of the branch and keep only the description of my commit), the PR history is a bit chaotic.

@mdevolde
Copy link
Collaborator

@jxmorris12 If I can help in any way to conclude the PR, please let me know ;)

@jxmorris12 jxmorris12 merged commit f394f57 into jxmorris12:master May 31, 2025
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants