-
Notifications
You must be signed in to change notification settings - Fork 12
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
Linter: Internalize functionality and scheduler integration stub #44
Conversation
Documentation for this branch can be viewed at https://sites.ecmwf.int/docs/loki/44 |
Codecov Report
📣 This organization is not using Codecov’s GitHub App Integration. We recommend you install it so Codecov can continue to function properly for your repositories. Learn more @@ Coverage Diff @@
## main #44 +/- ##
==========================================
+ Coverage 91.07% 91.65% +0.57%
==========================================
Files 86 86
Lines 15043 15206 +163
==========================================
+ Hits 13701 13937 +236
+ Misses 1342 1269 -73
Flags with carried forward coverage won't be shown. Click here to find out more.
... and 4 files with indirect coverage changes Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. |
I believe this is ready for a look. |
4f056f9
to
250ba62
Compare
6cd92b4
to
2f328e5
Compare
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.
Looks great, neat and tidy as always. Especially the Scheduler
changes to traversal and the file graph are much appreciated. CI and regression tests are looking good, so GTG from me.
nx.DiGraph | ||
""" | ||
paths = {item.path for item in self.item_graph} | ||
basepath = Path(commonpath([str(p) for p in paths])) |
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.
[no action] Neat. Might need to use this approach in Scheduler
in general.
Note: filing this early for test coverage and while testing offline.
This PR moves some of the functionality from
loki-lint.py
into theloki.lint
module to facilitate testing. In that process, some of the functionality could be replaced by internal utility functions. With this, we now have a new entry point for linting:lint_files
This PR also bundles a first stub of a scheduler integration for the linter. It implements linting as a transformation that is applied across the scheduler's graph.
However, since linting is file based, this required equipping the Scheduler with a file-centric view of dependencies. For that purpose, the scheduler has been equipped with the ability to generate a file graph for the scheduler's item_graph, and optionally use that in
process
. This may be handy for the FileWriteTransformation, too.The CLI script for checking in loki-lint.py is now only responsible for reading the YAML config and filling the config dict.