Skip to content

fix: replace mutable default arg and add encoding to open() calls#15493

Open
KeloYuan wants to merge 1 commit into
fastapi:masterfrom
KeloYuan:fix/mutable-defaults-and-encoding
Open

fix: replace mutable default arg and add encoding to open() calls#15493
KeloYuan wants to merge 1 commit into
fastapi:masterfrom
KeloYuan:fix/mutable-defaults-and-encoding

Conversation

@KeloYuan

@KeloYuan KeloYuan commented May 8, 2026

Copy link
Copy Markdown

Summary

This PR fixes two categories of code quality issues:

1. Mutable default argument in fastapi/_compat/v2.py

File: fastapi/_compat/v2.py

Replaced the mutable default argument values: dict[str, Any] = {} in ModelField.validate() with values: dict[str, Any] | None = None. Using a mutable object (like {}) as a default argument is a well-known Python pitfall (Pylint W0102 / flake8 B006) — the same dict instance is shared across all calls, which can lead to subtle bugs if the dict is ever mutated.

2. open() calls without explicit encoding in scripts/

File: scripts/add_latest_release_date.py

Added encoding='utf-8' to both open() calls (read and write). Without an explicit encoding, Python falls back to the platform locale default, which can differ between developer machines and CI environments (e.g. Windows uses cp1252 by default). Explicitly specifying UTF-8 ensures consistent behaviour everywhere (Pylint W1514).

Changes

  • fastapi/_compat/v2.py: values: dict[str, Any] = {}values: dict[str, Any] | None = None
  • scripts/add_latest_release_date.py: open(RELEASE_NOTES_FILE)open(RELEASE_NOTES_FILE, encoding='utf-8')
  • scripts/add_latest_release_date.py: open(RELEASE_NOTES_FILE, 'w')open(RELEASE_NOTES_FILE, 'w', encoding='utf-8')

…o open() calls

- fastapi/_compat/v2.py: Replace mutable default argument
  with  in ModelField.validate() to avoid
  shared state across calls (B006/W0102)

- scripts/add_latest_release_date.py: Add encoding='utf-8' to both open() calls
  to ensure consistent file reading/writing across platforms (W1514)
Copilot AI review requested due to automatic review settings May 8, 2026 16:50
@codspeed-hq

codspeed-hq Bot commented May 8, 2026

Copy link
Copy Markdown

Merging this PR will not alter performance

✅ 20 untouched benchmarks


Comparing KeloYuan:fix/mutable-defaults-and-encoding (12f149a) with master (fb74293)1

Open in CodSpeed

Footnotes

  1. No successful run was found on master (622b635) during the generation of this report, so fb74293 was used instead as the comparison base. There might be some changes unrelated to this pull request in this report.

Copilot AI left a comment

Copy link
Copy Markdown

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 addresses two cross-platform Python pitfalls in FastAPI’s codebase: avoiding mutable default arguments in a compatibility shim, and making a docs-maintenance script’s file I/O deterministic across locales by specifying UTF-8.

Changes:

  • Replaced a mutable default argument ({}) with None in ModelField.validate() in the Pydantic v2 compat layer.
  • Added encoding="utf-8" to the read and write open() calls in scripts/add_latest_release_date.py.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File Description
fastapi/_compat/v2.py Removes a mutable default arg from ModelField.validate() to avoid shared-state bugs and satisfy bugbear/ruff rules.
scripts/add_latest_release_date.py Makes script file reads/writes consistent across platforms by explicitly using UTF-8 encoding.

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

Comment thread scripts/add_latest_release_date.py
@YuriiMotov

Copy link
Copy Markdown
Member

@KeloYuan, thanks!

Let's only fix second issue (with encoding).

"Mutable default argument" is not an issue - this parameter is not used. This was probably done this way for for compatibility with Pydantic V1. It will likely be rewritten at some point.

Could you please update PR and its description to exclude the part for "Mutable default argument"?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

internal Internal changes waiting

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants