Eric Kafe

@HyperPS, have you considered the suggestions proposed in the review? You can commit each one by pressing a button, or reject it and give a reason.

Some suggestions are now outdated, so we need Copilot to update them.

The last review suggestion, about dropping the change in mapping.py, is not recommendable. This change keeps Windows CI green. Before switching to an explicit nltk: resource URL here, Windows failed...

Thanks @HyperPS, but first you'll need to follow the guidelines concerning pre-commit. Please see https://github.com/nltk/nltk/pull/3461#issuecomment-3518251102

The security fix is useful, but the implementation feels quite heavy for what it is trying to achieve. It might be possible to express the same checks in a more...

@HyperPS, this PR still has multiple problems. For ex., the changes to `_BOM_TABLE['utf16']` break existing behavior and introduce bugs: - `codecs.BOM16_BE` does not exist; the correct constant is `codecs.BOM_UTF16_BE`. Using...

Summary of remaining issues (overview) - _BOM_TABLE corruption - Ensure `_BOM_TABLE['utf16']` uses the standard `codecs.BOM_UTF16_LE` / `codecs.BOM_UTF16_BE` constants and remains a list of `(bom_bytes, new_encoding)` tuples. Do not return a...

Thanks @HyperPS! Some of the following tests still show remaining problems: [test_data_security.py](https://github.com/user-attachments/files/24026419/test_data_security.py)

The unit tests would pass if they just expected LookupError instead of ValueError. However, it is more helpful if find() and normalize_resource_url() raise ValueError instead, so that missing-resources and invalid-inputs...

The situation has improved after integrating #3484, and fixing non-Windows paths. Now only 2 failures remain, due to pytest expecting ValueError, where LookupError is raised instead. @HyperPS, note that all...