-
Notifications
You must be signed in to change notification settings - Fork 7
maint: fix code style, check on gha #252
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
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## develop #252 +/- ##
===========================================
+ Coverage 76.20% 76.24% +0.03%
===========================================
Files 34 34
Lines 3215 3216 +1
Branches 780 780
===========================================
+ Hits 2450 2452 +2
+ Misses 561 560 -1
Partials 204 204 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Instead of black and isort, run ruff as pre-commit hook. Enable pyupgrade and additional rules. Applied safe auto-fixes, but validation currently fails. Will be addressed in a follow-up PR (#252). Relevant changes: `.pre-commit-config.yaml`, `pyproject.toml`
dilpath
left a comment
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.
👍
| except KeyError: | ||
| raise KeyError( | ||
| f"Observable table missing mandatory field {OBSERVABLE_ID}." | ||
| ) | ||
| ) from None |
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.
as err, from err?
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'd prefer from None. We only want this simple message here without the pandas error on top.
| except KeyError: | ||
| raise KeyError( | ||
| f"Condition table missing mandatory field {CONDITION_ID}." | ||
| ) | ||
| ) from None |
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.
as err, from err?
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.
as previous
| ], | ||
| "quality": [ | ||
| "flake8>=3.8.3", | ||
| "pre-commit", |
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.
Is the ruff dependency somehow managed by pre-commit and the ruff-pre-commit repo?
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.
correct
| f"observableParameter1_{obs1_1_1_1}" | ||
| f" + observableParameter2_{obs1_1_1_1}", | ||
| f"observableParameter1_{obs1_2_1_1}" | ||
| f" + observableParameter2_{obs1_2_1_1}", | ||
| f"observableParameter1_{obs1_2_2_1}" | ||
| f" + observableParameter2_{obs1_2_2_1}", |
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.
😐
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.
we should have thought of something shorter than "observableParameter1_" 😅 🙈
Follow-up to #251