Skip to content

add strict zip options#331

Merged
gkuznik merged 1 commit intomasterfrom
update-deps
Mar 6, 2026
Merged

add strict zip options#331
gkuznik merged 1 commit intomasterfrom
update-deps

Conversation

@gkuznik
Copy link
Member

@gkuznik gkuznik commented Mar 6, 2026

update dependencies

update dependencies
Copilot AI review requested due to automatic review settings March 6, 2026 17:58
@gkuznik gkuznik enabled auto-merge (squash) March 6, 2026 17:59
@gkuznik gkuznik merged commit 2da1081 into master Mar 6, 2026
7 checks passed
@gkuznik gkuznik deleted the update-deps branch March 6, 2026 17:59
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR bumps the project’s Python/dependency baseline and adds strict-length checking to zip() calls to fail fast when upstream data shapes don’t match expectations.

Changes:

  • Raise the minimum supported Python version to >=3.10.
  • Update locked/runtime/dev dependencies (e.g., deepl, pytest, ruff, mypy, etc.).
  • Add zip(..., strict=True) in translation and menu parsing paths.

Reviewed changes

Copilot reviewed 3 out of 4 changed files in this pull request and generated 3 comments.

File Description
uv.lock Regenerates the lockfile for the updated dependency set and Python >=3.10.
pyproject.toml Bumps requires-python to >=3.10 and updates dependency constraints.
src/translate.py Uses strict zip() when mapping requested texts to DeepL results.
src/menu_parser.py Uses strict zip() when combining dish metadata lists during parsing.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +8 to +13
requires-python = ">= 3.10"
dependencies = [
"lxml~=6.0",
"pyopenmensa~=0.95",
"requests~=2.32",
"deepl>=1.2.1",
"deepl~=1.28",
Copy link

Copilot AI Mar 6, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

requires-python was bumped to >=3.10 here, but the repo’s .python-version file (used by CI via actions/setup-python) still targets ">=3.9" and the README still mentions Python 3.9+. Please update .python-version (and any user-facing docs) to avoid CI/runtime inconsistencies now that code uses Python 3.10-only features (e.g., zip(..., strict=True)).

Copilot uses AI. Check for mistakes.

result = self.translator.translate_text(to_translate, source_lang=self.source, target_lang=self.target)
for original, translation in zip(to_translate, result):
for original, translation in zip(to_translate, result, strict=True):
Copy link

Copilot AI Mar 6, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Using zip(..., strict=True) will raise ValueError if the DeepL client returns a different number of translations than inputs. main() currently only catches deepl.exceptions.DeepLException, so a strict-zip mismatch would crash the translation run and skip cache persistence (despite the comment about always updating the cache). Consider handling ValueError (or using a finally to always save the cache) so operational runs don’t lose progress when a mismatch is detected.

Suggested change
for original, translation in zip(to_translate, result, strict=True):
for original, translation in zip(to_translate, result):

Copilot uses AI. Check for mistakes.
Comment on lines 316 to 320
dish_markers_allergen,
dish_markers_type,
dish_markers_meatless,
strict=True,
):
Copy link

Copilot AI Mar 6, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The PR description only says "update dependencies", but this change also enforces strict-length checking in zips (and requires Python >=3.10). Please update the PR description to reflect the behavioral change (parsing/translation may now fail fast on upstream data shape mismatches) and the Python version bump.

Copilot uses AI. Check for mistakes.
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.

2 participants