Conversation
- Add deprecation warning when `name` parameter is omitted from Dataset - Update Dataset docstrings to indicate name will be required in future - When loading Dataset from file without name, use filename as default_name before calling constructor to avoid spurious warnings - Update all tests to provide explicit name parameter - Update documentation examples to include name parameter https://claude.ai/code/session_01AKfpBCFjTPWEJdKu5oLKKH
Docs Preview
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4a7c58d422
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if name is None: | ||
| warnings.warn( | ||
| 'Omitting the `name` parameter is deprecated. ' | ||
| 'Please provide a name for your Dataset.', | ||
| DeprecationWarning, | ||
| stacklevel=2, | ||
| ) |
There was a problem hiding this comment.
Scope deprecation warning to avoid hard failures under -W error
Dataset.__init__ now unconditionally emits a DeprecationWarning whenever name is None, and this warning becomes an exception in environments that treat warnings as errors (including this repo’s pytest config with filterwarnings = ["error", ...]). That means existing unnamed-dataset code paths (for example current tests like tests/evals/test_report_evaluators.py) now fail at construction time instead of exercising behavior, so this change is effectively breaking rather than deprecating for strict-warning users. Consider limiting the warning to explicit end-user constructor calls or updating all remaining internal unnamed call sites in the same change.
Useful? React with 👍 / 👎.
| if name is None: | ||
| warnings.warn( | ||
| 'Omitting the `name` parameter is deprecated. ' | ||
| 'Please provide a name for your Dataset.', | ||
| DeprecationWarning, | ||
| stacklevel=2, | ||
| ) |
There was a problem hiding this comment.
🚩 Incomplete deprecation migration across docs
The PR updates name in a subset of doc files (quick-start, core-concepts, overview, report-evaluators), but dozens of other doc files still have Dataset(cases=...) without name — e.g., docs/evals/evaluators/llm-judge.md:384, docs/evals/how-to/concurrency.md:19, docs/evals/how-to/retry-strategies.md:33, docs/evals/how-to/metrics-attributes.md:121, docs/evals.md:150, pydantic_evals/README.md:47, and pydantic_evals/pydantic_evals/lifecycle.py:59 (docstring). The test_examples.py:132 filter 'ignore:Omitting the \name` parameter is deprecated'ensures tests pass, but perdocs/AGENTS.md` rule:714 ("Omit deprecated features from user-facing docs"), these remaining usages should ideally be updated in a follow-up to avoid teaching users the deprecated pattern.
Was this helpful? React with 👍 or 👎 to provide feedback.
Use a `UserWarning` subclass instead of `DeprecationWarning` so the deprecation is visible by default at runtime, following the approach from sethmlarson.dev/deprecations-via-warnings-dont-work-for-python-libraries. Also pass `name` to `Dataset(...)` in test_report_evaluators to stop the warning from being promoted to an error by pytest's `filterwarnings = ["error"]` config.
7a6afc9 to
13c41b2
Compare
…slim tests The class-based `ignore::pydantic_evals.PydanticEvalsDeprecationWarning` filter crashes xdist workers when pydantic_evals is not installed.
nameparameter is omitted from Dataset