-
Notifications
You must be signed in to change notification settings - Fork 11
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Update Copier Template to 2.2.0 #737
Conversation
@@ -16,7 +16,7 @@ def test_group_uid(group: str): | |||
|
|||
|
|||
def test_type_checking_ignores_inject(): | |||
def example_function(x: Movable = inject("foo")) -> MsgGenerator: | |||
def example_function(x: Movable = inject("foo")) -> MsgGenerator: # noqa: B008 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"I", # isort - https://docs.astral.sh/ruff/rules/#isort-i | ||
"W", # pycodestyle warnings - https://beta.ruff.rs/docs/rules/#warning-w | ||
"UP", # pyupgrade - https://docs.astral.sh/ruff/rules/#pyupgrade-up | ||
"I001", # isort |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure why isort was left there by the previous template upgrade, I assume it is now fully replaced by ruff. Happy to put it back if I'm wrong though.
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #737 +/- ##
=======================================
Coverage 94.43% 94.43%
=======================================
Files 112 112
Lines 4531 4531
=======================================
Hits 4279 4279
Misses 252 252 ☔ View full report in Codecov by Sentry. |
cb9454b
to
0e914fd
Compare
@@ -31,7 +31,7 @@ def trigger(self): | |||
|
|||
def _acq_done(*args, **kwargs): | |||
# TODO sort out if anything useful in here | |||
self._status._finished() | |||
self._status._finished() # noqa: SLF001 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is causing a coverage warning because it is untested, however it also appears to be dead code. along with the entire areadetector
package. The only place it is used in the adsim
module, which doesn't see much use and it can use the ophyd-async
detector these. It may be easier to just delete this package.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Am I right this will get removed in #404?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not will, but could, yes!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It has now been removed as part of #405, I didn't realise how much dead code was in that dir
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great, thank you!
0e914fd
to
0aa96b8
Compare
@DominicOram thanks for rebasing, are you happy to merge with the lower coverage or do you want to wait for #404? |
Fixes #736
Instructions to reviewer on how to test:
_
in thesrc
directoryChecks for reviewer
dodal connect ${BEAMLINE}