From 3a5571810c85ba3fa0de990309a6925839b128a8 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sun, 22 Mar 2026 16:09:22 +0000 Subject: [PATCH 1/3] Initial plan From f1f57f860c403a35f44df5cacee2d51944364b81 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sun, 22 Mar 2026 16:19:25 +0000 Subject: [PATCH 2/3] Fix exception forwarding, normalize messages, add warning/exception conventions to CONTRIBUTING.md Co-authored-by: bact <128572+bact@users.noreply.github.com> Agent-Logs-Url: https://github.com/PyThaiNLP/pythainlp/sessions/1a240bb9-52d0-40e6-8739-79cf52fa9e65 --- CONTRIBUTING.md | 132 ++++++++++++++++++++++++++++ pythainlp/cli/benchmark.py | 7 +- pythainlp/corpus/core.py | 12 ++- pythainlp/parse/attaparse_engine.py | 6 +- pythainlp/parse/esupar_engine.py | 6 +- pythainlp/phayathaibert/core.py | 4 +- pythainlp/spell/phunspell.py | 6 +- pythainlp/spell/symspellpy.py | 6 +- pythainlp/spell/tltk.py | 6 +- pythainlp/tag/pos_tag.py | 6 +- pythainlp/tag/thai_nner.py | 4 +- pythainlp/tag/tltk.py | 6 +- pythainlp/tokenize/deepcut.py | 6 +- pythainlp/tokenize/tltk.py | 6 +- pythainlp/tools/misspell.py | 4 +- pythainlp/translate/en_th.py | 12 +-- pythainlp/transliterate/tltk.py | 6 +- pythainlp/util/abbreviation.py | 12 ++- pythainlp/wangchanberta/core.py | 6 +- 19 files changed, 194 insertions(+), 59 deletions(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index aafc44bcf..62af1fa76 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -53,6 +53,138 @@ Please refer to our [empty-line]: https://stackoverflow.com/questions/5813311/no-newline-at-end-of-file#5813359 [posix]: https://stackoverflow.com/questions/729692/why-should-text-files-end-with-a-newline +### Error messages, warnings, and exception handling + +Clear, consistent, and parseable error and warning messages help users +debug problems and allow tooling to parse and filter output. +Follow these conventions in all PyThaiNLP code. + +#### Exception types + +Use the most specific built-in exception type for each situation: + +| Situation | Exception type | +| --- | --- | +| Missing optional dependency at import time | `ImportError` or `ModuleNotFoundError` | +| Invalid argument value | `ValueError` | +| Wrong argument type | `TypeError` | +| Required file not found | `FileNotFoundError` | +| I/O or OS-level failure | `OSError` | +| Runtime failure with no more-specific type | `RuntimeError` | +| Feature not yet implemented | `NotImplementedError` | + +#### Exception message format + +Write messages as complete sentences. + +- End each sentence with a period. +- Identify the offending value, name, or path explicitly. +- For a missing optional dependency, include the `pip install` command: + + ``` + is not installed. Install it with: pip install + ``` + + If the package is a PyThaiNLP optional-dependency group, name the extra: + + ``` + is required for this feature. + Install it with: pip install pythainlp[] + ``` + +- For an invalid argument value, name the parameter and show the received value: + + ``` + must be ; got {value!r} + ``` + +#### Exception forwarding (chaining) + +Always chain exceptions with `from` when raising a new exception inside an +`except` block, so the full traceback and original cause are preserved: + +```python +# Correct: chain to the original exception. +try: + import some_package +except ImportError as e: + raise ImportError( + "some_package is not installed. Install it with: pip install some_package" + ) from e + +# Correct: suppress chain with `from None` only when the original +# exception is irrelevant noise that would confuse the user. +raise ValueError("Invalid configuration value.") from None +``` + +Never use `raise e` to re-raise a caught exception. Use bare `raise` instead, +which preserves the original traceback: + +```python +# Correct: bare re-raise preserves traceback. +except Exception: + log_or_print_something() + raise + +# Incorrect: `raise e` creates a new traceback starting at this line. +except Exception as e: + log_or_print_something() + raise e # do not do this +``` + +When re-raising as a *different* exception type, always supply `from`: + +```python +# Correct. +except OSError as e: + raise RuntimeError(f"Failed to read model file: {e}") from e + +# Incorrect: original cause is silently discarded. +except OSError as e: + raise RuntimeError(f"Failed to read model file: {e}") +``` + +#### Warnings + +Use `warnings.warn()` for non-fatal conditions that the caller should know +about. Always pass both `category` and `stacklevel` explicitly: + +```python +import warnings + +warnings.warn("Message.", UserWarning, stacklevel=2) +``` + +Recommended categories: + +| Situation | Category | +| --- | --- | +| Deprecated API that will be removed in a future version | `DeprecationWarning` | +| Skipped or degraded behavior the caller should review | `UserWarning` | + +Use `stacklevel=2` so the warning points at the **caller's** line, not the +internal line that called `warnings.warn()`. Increase `stacklevel` by one for +each additional layer of indirection between the public API and the +`warnings.warn()` call. + +Warning messages should be clear, concise, and parseable: + +- Write messages as complete sentences ending with a period. +- For deprecations, name the deprecated symbol and its replacement: + + ``` + is deprecated; use instead. + ``` + +- For skipped data or fallback behavior, describe what was skipped and why: + + ``` + Skipping entry with : {value!r} + ``` + +Do **not** rely on the default `UserWarning` by omitting `category`. +Always supply `category` explicitly for clarity and greppability. + ### Version Control System - We use [Git](https://git-scm.com/) as our diff --git a/pythainlp/cli/benchmark.py b/pythainlp/cli/benchmark.py index 595551e1d..953bb8181 100644 --- a/pythainlp/cli/benchmark.py +++ b/pythainlp/cli/benchmark.py @@ -95,10 +95,11 @@ def __init__(self, name: str, argv: Sequence[str]) -> None: import yaml from pythainlp.benchmarks import word_tokenization - except ImportError: + except ImportError as e: raise ImportError( - "Please install the extra dependencies `benchmarks` to use this command by running `pip install pythainlp[benchmarks]`" - ) + "The 'benchmarks' extra dependencies are required for this command." + " Install them with: pip install pythainlp[benchmarks]" + ) from e df_raw = word_tokenization.benchmark(expected, actual) diff --git a/pythainlp/corpus/core.py b/pythainlp/corpus/core.py index 058408a3d..8055d8d04 100644 --- a/pythainlp/corpus/core.py +++ b/pythainlp/corpus/core.py @@ -484,7 +484,7 @@ def _safe_extract_tar(tar: tarfile.TarFile, path: str) -> None: tarfile.LinkOutsideDestinationError, ) as e: # Re-raise as ValueError for consistency with older Python versions - raise ValueError(str(e)) + raise ValueError(str(e)) from e else: # Manual validation for older Python versions for member in tar.getmembers(): @@ -900,13 +900,11 @@ def get_hf_hub(repo_id: str, filename: str = "") -> str: """ try: from huggingface_hub import hf_hub_download, snapshot_download - except ModuleNotFoundError: + except ModuleNotFoundError as e: raise ModuleNotFoundError( - """ - huggingface-hub isn't found! - Please installing the package via 'pip install huggingface-hub'. - """ - ) + "huggingface-hub is not installed." + " Install it with: pip install huggingface-hub" + ) from e except Exception as e: raise RuntimeError(f"An unexpected error occurred: {e}") from e hf_root = get_full_data_path("hf_models") diff --git a/pythainlp/parse/attaparse_engine.py b/pythainlp/parse/attaparse_engine.py index 1cc0b383f..f6a66d286 100644 --- a/pythainlp/parse/attaparse_engine.py +++ b/pythainlp/parse/attaparse_engine.py @@ -12,10 +12,10 @@ try: from attaparse import depparse, load_model -except ImportError: +except ImportError as e: raise ImportError( - "Import Error; Install attaparse by pip install attaparse" - ) + "attaparse is not installed. Install it with: pip install attaparse" + ) from e if TYPE_CHECKING: from stanza import Pipeline diff --git a/pythainlp/parse/esupar_engine.py b/pythainlp/parse/esupar_engine.py index a6b00132e..489c31c31 100644 --- a/pythainlp/parse/esupar_engine.py +++ b/pythainlp/parse/esupar_engine.py @@ -9,8 +9,10 @@ try: import esupar -except ImportError: - raise ImportError("Import Error; Install esupar by pip install esupar") +except ImportError as e: + raise ImportError( + "esupar is not installed. Install it with: pip install esupar" + ) from e if TYPE_CHECKING: from esupar import Model diff --git a/pythainlp/phayathaibert/core.py b/pythainlp/phayathaibert/core.py index 036ebee25..d6485b7fc 100644 --- a/pythainlp/phayathaibert/core.py +++ b/pythainlp/phayathaibert/core.py @@ -407,8 +407,8 @@ def get_ner( if pos: warnings.warn( - "This model doesn't support output \ - postag and It doesn't output the postag.", + "This model does not support POS tag output.", + UserWarning, stacklevel=2, ) diff --git a/pythainlp/spell/phunspell.py b/pythainlp/spell/phunspell.py index 6b5ee6fe4..c1b886785 100644 --- a/pythainlp/spell/phunspell.py +++ b/pythainlp/spell/phunspell.py @@ -19,10 +19,10 @@ try: import phunspell -except ImportError: +except ImportError as e: raise ImportError( - "Import Error; Install phunspell by pip install phunspell" - ) + "phunspell is not installed. Install it with: pip install phunspell" + ) from e pspell: "phunspell.Phunspell" = phunspell.Phunspell("th_TH") diff --git a/pythainlp/spell/symspellpy.py b/pythainlp/spell/symspellpy.py index 54ee3246f..c592147d3 100644 --- a/pythainlp/spell/symspellpy.py +++ b/pythainlp/spell/symspellpy.py @@ -22,10 +22,10 @@ try: from symspellpy import SymSpell, Verbosity -except ImportError: +except ImportError as e: raise ImportError( - "Import Error; Install symspellpy by pip install symspellpy" - ) + "symspellpy is not installed. Install it with: pip install symspellpy" + ) from e from pythainlp.corpus import get_corpus_path diff --git a/pythainlp/spell/tltk.py b/pythainlp/spell/tltk.py index 9fce54771..13cdb5b97 100644 --- a/pythainlp/spell/tltk.py +++ b/pythainlp/spell/tltk.py @@ -16,10 +16,10 @@ try: from tltk.nlp import spell_candidates -except ImportError: +except ImportError as e: raise ImportError( - "Not found tltk! Please install tltk by pip install tltk" - ) + "tltk is not installed. Install it with: pip install tltk" + ) from e def spell(text: str) -> list[str]: diff --git a/pythainlp/tag/pos_tag.py b/pythainlp/tag/pos_tag.py index 6fbc3f2f0..47fcf641a 100644 --- a/pythainlp/tag/pos_tag.py +++ b/pythainlp/tag/pos_tag.py @@ -216,10 +216,10 @@ def pos_tag_transformers( AutoTokenizer, TokenClassificationPipeline, ) - except ImportError: + except ImportError as e: raise ImportError( - "Not found transformers! Please install transformers by pip install transformers" - ) + "transformers is not installed. Install it with: pip install transformers" + ) from e if not sentence: return [] diff --git a/pythainlp/tag/thai_nner.py b/pythainlp/tag/thai_nner.py index ef2d683b1..eb07506e2 100644 --- a/pythainlp/tag/thai_nner.py +++ b/pythainlp/tag/thai_nner.py @@ -141,10 +141,10 @@ def __init__(self, path_model: Optional[str] = None) -> None: # 3. Clear error message only when ThaiNNER class is actually instantiated try: from thai_nner import NNER - except ImportError: + except ImportError as e: raise ImportError( "thai-nner library not found. Please install it with 'pip install thai-nner'." - ) + ) from e self.model: NNER = NNER(path_model=path_model) def tag( diff --git a/pythainlp/tag/tltk.py b/pythainlp/tag/tltk.py index 64caa265a..ebcf8937f 100644 --- a/pythainlp/tag/tltk.py +++ b/pythainlp/tag/tltk.py @@ -7,10 +7,10 @@ try: from tltk import nlp -except ImportError: +except ImportError as e: raise ImportError( - "Not found tltk! Please install tltk by pip install tltk" - ) + "tltk is not installed. Install it with: pip install tltk" + ) from e from pythainlp.tokenize import word_tokenize nlp.pos_load() diff --git a/pythainlp/tokenize/deepcut.py b/pythainlp/tokenize/deepcut.py index ba397ab17..611361a6c 100644 --- a/pythainlp/tokenize/deepcut.py +++ b/pythainlp/tokenize/deepcut.py @@ -16,8 +16,10 @@ try: from deepcut import tokenize -except ImportError: - raise ImportError("Please install deepcut by pip install deepcut") +except ImportError as e: + raise ImportError( + "deepcut is not installed. Install it with: pip install deepcut" + ) from e from pythainlp.util import Trie diff --git a/pythainlp/tokenize/tltk.py b/pythainlp/tokenize/tltk.py index b345ee47a..8db3060bd 100644 --- a/pythainlp/tokenize/tltk.py +++ b/pythainlp/tokenize/tltk.py @@ -6,10 +6,10 @@ try: from tltk.nlp import syl_segment from tltk.nlp import word_segment as tltk_segment -except ImportError: +except ImportError as e: raise ImportError( - "Not found tltk! Please install tltk by pip install tltk" - ) + "tltk is not installed. Install it with: pip install tltk" + ) from e def segment(text: str) -> list[str]: diff --git a/pythainlp/tools/misspell.py b/pythainlp/tools/misspell.py index 07c736f50..a805f2a89 100644 --- a/pythainlp/tools/misspell.py +++ b/pythainlp/tools/misspell.py @@ -110,9 +110,9 @@ def find_misspell_candidates( printing_locations[ix] = char except IndexError: continue - except Exception as e: + except Exception: print("Something wrong with: ", char) - raise e + raise return chars diff --git a/pythainlp/translate/en_th.py b/pythainlp/translate/en_th.py index 36f92efd7..c2f0d4763 100644 --- a/pythainlp/translate/en_th.py +++ b/pythainlp/translate/en_th.py @@ -16,17 +16,17 @@ try: from fairseq.models.transformer import TransformerModel -except ImportError: +except ImportError as e: raise ImportError( - "Not found fairseq! Please install fairseq by pip install fairseq" - ) + "fairseq is not installed. Install it with: pip install fairseq" + ) from e try: from sacremoses import MosesTokenizer -except ImportError: +except ImportError as e: raise ImportError( - "Not found sacremoses! Please install sacremoses by pip install sacremoses" - ) + "sacremoses is not installed. Install it with: pip install sacremoses" + ) from e from pythainlp.corpus import download, get_corpus_path diff --git a/pythainlp/transliterate/tltk.py b/pythainlp/transliterate/tltk.py index 852e4a912..28bdf4a0b 100644 --- a/pythainlp/transliterate/tltk.py +++ b/pythainlp/transliterate/tltk.py @@ -7,10 +7,10 @@ try: from tltk.nlp import g2p, th2ipa, th2roman -except ImportError: +except ImportError as e: raise ImportError( - "Not found tltk! Please install tltk by pip install tltk" - ) + "tltk is not installed. Install it with: pip install tltk" + ) from e def romanize(text: str) -> str: diff --git a/pythainlp/util/abbreviation.py b/pythainlp/util/abbreviation.py index 85d3e443b..73765f829 100644 --- a/pythainlp/util/abbreviation.py +++ b/pythainlp/util/abbreviation.py @@ -36,12 +36,10 @@ def abbreviation_to_full_text( """ try: from khamyo import replace as _replace - except ImportError: + except ImportError as e: raise ImportError( - """ - This function needs to use khamyo. - You can install by pip install khamyo or - pip install pythainlp[abbreviation]. - """ - ) + "khamyo is required for this feature." + " Install it with: pip install khamyo" + " or pip install pythainlp[abbreviation]" + ) from e return cast(list[tuple[str, Optional[float]]], _replace(text, top_k=top_k)) diff --git a/pythainlp/wangchanberta/core.py b/pythainlp/wangchanberta/core.py index 40a9aeec1..cb1474381 100644 --- a/pythainlp/wangchanberta/core.py +++ b/pythainlp/wangchanberta/core.py @@ -98,7 +98,8 @@ def get_ner( """ if pos: warnings.warn( - "This model doesn't support output of POS tags and it doesn't output the POS tags.", + "This model does not support POS tag output.", + UserWarning, stacklevel=2, ) text = re.sub(" ", "<_>", text) @@ -217,7 +218,8 @@ def get_ner( if pos: warnings.warn( - "This model doesn't support output postag and It doesn't output the postag.", + "This model does not support POS tag output.", + UserWarning, stacklevel=2, ) words_token = word_tokenize(text.replace(" ", "<_>")) From 49050107d81aa93e41ef93a6f2e8f24e9aaaaf64 Mon Sep 17 00:00:00 2001 From: Arthit Suriyawongkul Date: Sun, 22 Mar 2026 17:32:54 +0000 Subject: [PATCH 3/3] Update CONTRIBUTING.md --- CONTRIBUTING.md | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 62af1fa76..d9fcc2699 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -81,20 +81,20 @@ Write messages as complete sentences. - Identify the offending value, name, or path explicitly. - For a missing optional dependency, include the `pip install` command: - ``` + ```text is not installed. Install it with: pip install ``` If the package is a PyThaiNLP optional-dependency group, name the extra: - ``` + ```text is required for this feature. Install it with: pip install pythainlp[] ``` - For an invalid argument value, name the parameter and show the received value: - ``` + ```text must be ; got {value!r} ``` @@ -172,13 +172,13 @@ Warning messages should be clear, concise, and parseable: - Write messages as complete sentences ending with a period. - For deprecations, name the deprecated symbol and its replacement: - ``` + ```text is deprecated; use instead. ``` - For skipped data or fallback behavior, describe what was skipped and why: - ``` + ```text Skipping entry with : {value!r} ```