Skip to content

[ty] Install more dependencies before benchmarking hydra-zen#18757

Closed
AlexWaygood wants to merge 1 commit intomainfrom
alex/more-hydra-deps
Closed

[ty] Install more dependencies before benchmarking hydra-zen#18757
AlexWaygood wants to merge 1 commit intomainfrom
alex/more-hydra-deps

Conversation

@AlexWaygood
Copy link
Member

Summary

This makes the number of unresolved-import diagnostics on hydra-zen go down from 136 to 16. The overall number of diagnostics goes down from 703 to 588.

That seems worth the cost of installing them considering that hydra-zen would likely have these imports installed if they were running ty in CI, and we want the benchmark to accurately mimic the experience users would have if they ran ty on these projects in CI. (In CI, gathering suggestions for an unresolved-import diagnostic would usually be a very cold path, for example, because there would usually be a very small number of diagnostics like this.)

Test Plan

I:

  1. Cloned hydra-zen
  2. Created a virtual environment at <hydra-zen repo root>/.venv
  3. Ran `uv pip install pydantic beartype hydra-core
  4. Ran path/to/ty/binary check and observed that there were 136 `unresolved-import diagnostics
  5. Ran uv pip install pytest hypothesis
  6. Ran path/to/ty/binary check and observed that there were now only 16 `unresolved-import diagnostics

@AlexWaygood AlexWaygood requested a review from MichaReiser June 18, 2025 14:54
@AlexWaygood AlexWaygood added testing Related to testing Ruff itself ty Multi-file analysis & type inference labels Jun 18, 2025
@MichaReiser
Copy link
Member

You might want to update mypy primer too 😅

Hydra is already one of the slowest to install. I'm curious by how much performance degrades due to adding the new dependencies.

@AlexWaygood
Copy link
Member Author

You might want to update mypy primer too 😅

I will!

@github-actions
Copy link
Contributor

ruff-ecosystem results

Linter (stable)

✅ ecosystem check detected no linter changes.

Linter (preview)

✅ ecosystem check detected no linter changes.

commit: "dd2b50a9614c6f8c46c5866f283c8f7e7a960aa8",
paths: vec![SystemPath::new("src")],
dependencies: vec!["pydantic", "beartype", "hydra-core"],
dependencies: vec!["pydantic", "beartype", "hydra-core", "pytest", "hypothesis"],
Copy link
Member

Choose a reason for hiding this comment

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

I can't comment but you might want to lower the number of the max expected diagnostics on line 441.

That makes me realise that we may want to enable all rules, including possibly-* rules

Copy link
Member Author

Choose a reason for hiding this comment

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

That makes me realise that we may want to enable all rules, including possibly-* rules

that makes sense but I'll leave it out of this PR

Copy link
Member

Choose a reason for hiding this comment

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

Oh yeah, that was more a note for myself

@AlexWaygood
Copy link
Member Author

I realised that the benchmarks only run ty on the src/ directory, and that pytest/hypothesis are not needed for checking those directories. So this PR isn't necessary.

@AlexWaygood AlexWaygood deleted the alex/more-hydra-deps branch June 18, 2025 15:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

testing Related to testing Ruff itself ty Multi-file analysis & type inference

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

Comments