DiamondLightSource / tickit

Event-based hardware simulation framework
Apache License 2.0
6 stars 0 forks source link

Fix type errors caught by pyright but not mypy #156

Closed tpoliaw closed 1 year ago

tpoliaw commented 1 year ago

Where type checking has been ignored, comments have been added to explain why the code is still valid.

codecov[bot] commented 1 year ago

Codecov Report

Merging #156 (a7dc12d) into master (6066902) will decrease coverage by 0.08%. The diff coverage is 93.54%.

@@            Coverage Diff             @@
##           master     #156      +/-   ##
==========================================
- Coverage   94.50%   94.42%   -0.08%     
==========================================
  Files          44       44              
  Lines        1292     1310      +18     
==========================================
+ Hits         1221     1237      +16     
- Misses         71       73       +2     
Impacted Files Coverage Δ
src/tickit/adapters/epicsadapter/ioc_manager.py 59.37% <0.00%> (ø)
src/tickit/core/state_interfaces/kafka.py 83.33% <0.00%> (-2.88%) :arrow_down:
src/tickit/utils/configuration/tagged_union.py 87.23% <ø> (ø)
...kit/adapters/interpreters/command/regex_command.py 96.15% <94.44%> (-3.85%) :arrow_down:
src/tickit/adapters/epicsadapter/adapter.py 100.00% <100.00%> (ø)
...apters/interpreters/command/command_interpreter.py 96.77% <100.00%> (+0.10%) :arrow_up:
src/tickit/adapters/interpreters/utils.py 100.00% <100.00%> (ø)
...pters/interpreters/wrappers/joining_interpreter.py 100.00% <100.00%> (ø)
...ers/interpreters/wrappers/splitting_interpreter.py 100.00% <100.00%> (ø)
...rc/tickit/core/state_interfaces/state_interface.py 88.23% <100.00%> (ø)
... and 3 more
tpoliaw commented 1 year ago

There are still a few (6) errors remaining but mostly around the interpreters which I'd like to overhaul slightly anyway

tpoliaw commented 1 year ago

Up to you but I think I'd go for a separate PR so it can be rolled back more easily if there's a pyright problem we can't work around. Everything in this PR should still be valid if we stick with mypy.

abbiemery commented 1 year ago

Agree. In which case either resolve the issue in the push device or make a new issue for it, and merge this :)

tpoliaw commented 1 year ago

make a new issue for it

170