-
Notifications
You must be signed in to change notification settings - Fork 11
Refactor workload.py #171
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
base: main
Are you sure you want to change the base?
Refactor workload.py #171
Conversation
james-bruten-mo
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.
Nice use of the classmethod!
A couple of minor suggestions
| self.test = test | ||
|
|
||
| @classmethod | ||
| def from_github(cls, capture: bool = False, file: Path = None) -> "ProjectData": |
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.
| def from_github(cls, capture: bool = False, file: Path = None) -> "ProjectData": | |
| def from_github(cls, capture: bool = False, file: Path = None) -> ProjectData: |
And then you'll need from __future__ import annotations. You actually don't need the import at python 14.0, but we're not commonly using that yet
| Retrieve data from GitHub API and initialise the class. | ||
| """ | ||
| command = "gh project item-list 376 -L 500 --owner MetOffice --format json" | ||
| output = subprocess.run(command.split(), capture_output=True, timeout=180) |
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.
| output = subprocess.run(command.split(), capture_output=True, timeout=180) | |
| output = subprocess.run(shlex.split(command), capture_output=True, timeout=180) |
and import shlex - I learnt this one recently, it's great!
| store it in a dictionary keyed by repository. | ||
| """ | ||
|
|
||
| data = {} |
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.
| data = {} | |
| data = defaultdict(list) |
with from collections import defaultdict
And then below there's no need to check if repo is in data. Just do data[repo].append(pull_request) and it'll create it
PR Summary
Sci/Tech Reviewer:
Code Reviewer: @james-bruten-mo
I've pulled the code for interacting with a github project into its own module as I would like to use this for a new script too. I've refactored workload.py to use this, and modified the interface to hopefully make it a little more generic. I've also removed the hardcoded list of repositories from workload.py, instead now including columns in the SSD table for all repos that have PRs in the project so that everything is included. This is lots at the moment as we had some wide-reaching workflow updates, but hopefully in the future this will actually reduce the size of the table to not include empty columns.
I've also updated the repository list in the manage milestones list to be more complete and fixed a bug with "'s.
Code Quality Checklist
Testing
Security Considerations
AI Assistance and Attribution
Sci/Tech Review
(Please alert the code reviewer via a tag when you have approved the SR)
Code Review