Skip to content
This repository has been archived by the owner on Sep 2, 2024. It is now read-only.

Check diffs with Pyright #1042

Closed
d-perl opened this issue Dec 13, 2023 · 1 comment · Fixed by #1043
Closed

Check diffs with Pyright #1042

d-perl opened this issue Dec 13, 2023 · 1 comment · Fixed by #1043
Assignees

Comments

@d-perl
Copy link
Contributor

d-perl commented Dec 13, 2023

Using https://github.com/Bachmann1234/diff_cover
And this plugin https://github.com/dperl-dls/pyright_diff_quality_plugin
We can test branches for Pyright type-checking compliance without having to fix everything in the codebase at once.

To test this out:

  • Install both tools into a venv with hyperion
  • switch to some branch which you think or know has errors
  • run diff-quality --violations pyright

For example, on my current branch I get:

-------------
Diff Quality
Quality Report: pyright
Diff: origin/main...HEAD, staged and unstaged changes
-------------
conftest.py (100%)
src/__init__.py (100%)
src/hyperion/__main__.py (100%)
src/hyperion/external_interaction/callbacks/__init__.py (100%)
src/hyperion/external_interaction/callbacks/__main__.py (100%)
src/hyperion/external_interaction/callbacks/ispyb_callback_base.py (100%)
src/hyperion/external_interaction/callbacks/plan_reactive_callback.py (100%)
src/hyperion/external_interaction/callbacks/rotation/zocalo_callback.py (100%)
src/hyperion/external_interaction/callbacks/xray_centre/zocalo_callback.py (100%)
src/hyperion/external_interaction/zocalo/zocalo_interaction.py (85.7%):
src/hyperion/external_interaction/zocalo/zocalo_interaction.py:120: Object of type "None" is not subscriptable
src/hyperion/log.py (100%)
src/hyperion/parameters/cli.py (100%)
src/hyperion/parameters/constants.py (100%)
tests/system_tests/hyperion/test_main_system.py (100%)
tests/unit_tests/experiment_plans/conftest.py (100%)
tests/unit_tests/experiment_plans/test_flyscan_xray_centre_plan.py (100%)
tests/unit_tests/external_interaction/callbacks/test_external_callbacks.py (100%)
tests/unit_tests/external_interaction/callbacks/test_plan_reactive_callback.py (94.4%):
tests/unit_tests/external_interaction/callbacks/test_plan_reactive_callback.py:71: Cannot access member "assert_called_once" for type "function"
  Member "assert_called_once" is unknown
tests/unit_tests/external_interaction/conftest.py (100%)
-------------
Total:   400 lines
Violations: 2 lines
% Quality: 99%
-------------

Should we use this to help us migrate slowly to stricter type checking (and at some stage, hopefully, discard it for just using pyright)?

@DominicOram DominicOram moved this to Backlog in Hyperion Dec 13, 2023
@DominicOram
Copy link
Collaborator

DominicOram commented Dec 13, 2023

I really like this, thank you! I think we should use it. Some things we should do first though:

  • As you've said in Add support for Pyright as a quality-checking tool Bachmann1234/diff_cover#379 we should get it into the original repo. My preference would be to make a fork into the DLS org with this in then get it merged upstream when we can
  • We should think about how to get it in our CI. As a first pass getting it into the PR checks would be good. It would be good to get it into the pre-commit but at the moment it takes about 10-15s to run on one of my branches. Can we get it to run on the diff of staged changes and how long does it take in this case?
  • We should advertise it round the rest of DLS

d-perl added a commit that referenced this issue Dec 14, 2023
d-perl added a commit that referenced this issue Dec 14, 2023
d-perl added a commit that referenced this issue Dec 14, 2023
d-perl added a commit that referenced this issue Dec 14, 2023
@DominicOram DominicOram moved this from Backlog to Review in Hyperion Dec 14, 2023
@DominicOram DominicOram moved this from Review to In Progress in Hyperion Dec 14, 2023
d-perl added a commit that referenced this issue Dec 14, 2023
@DominicOram DominicOram moved this from In Progress to Review in Hyperion Dec 14, 2023
@DominicOram DominicOram moved this from Review to In Progress in Hyperion Dec 14, 2023
@DominicOram DominicOram moved this from In Progress to Review in Hyperion Dec 15, 2023
@github-project-automation github-project-automation bot moved this from Review to Done in Hyperion Dec 15, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

2 participants