-
-
Notifications
You must be signed in to change notification settings - Fork 25
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 production tests #578
Conversation
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.
a single suggestion. the case could be made to either implement it or keep it as-is.
love your aesthetic, it's pleasing to read.
@@ -62,37 +64,40 @@ def available(self) -> bool: | |||
except: | |||
return False | |||
|
|||
RCA_INPUTS = { sid: 996 + sid for sid in range(models.MAX_SOURCES) } | |||
|
|||
RCA_INPUTS = {sid: 996 + sid for sid in range(models.MAX_SOURCES)} |
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.
suggestion: it may (or may not) make more sense to from amplipi.defaults import RCAs
and use that instead of this.
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'll take a look at that. I don't think that existed when Lincoln (or much less likely I) wrote this. This change is only popping up here because of auto-format actually kicking in now that I fixed the things VS Code has been yelling at us about for weeks months years.
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 definitely did not exist before a month ago even. you could definitely leave it for future refactor, np.
Apparently I broke mypy with a function attribute so hang tight while I blow everything up into a full on class... |
Codecov ReportAttention:
❗ Your organization needs to install the Codecov GitHub app to enable full functionality. Additional details and impacted files@@ Coverage Diff @@
## develop #578 +/- ##
===========================================
+ Coverage 51.53% 51.60% +0.07%
===========================================
Files 24 24
Lines 5676 5676
===========================================
+ Hits 2925 2929 +4
+ Misses 2751 2747 -4
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
tests.py