Skip to content
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

Move core logic out of __init__.py #117

Open
sadielbartholomew opened this issue Oct 1, 2024 · 0 comments
Open

Move core logic out of __init__.py #117

sadielbartholomew opened this issue Oct 1, 2024 · 0 comments

Comments

@sadielbartholomew
Copy link
Member

sadielbartholomew commented Oct 1, 2024

We have a lot of the core logic, including CLI processing, contained presently in the __init__.py file, but since the main function of this file is to initialise the module rather than hold functional logic, I argue we should move at least some of this out to either existing, or new, other modules of the codebase.

Notably, there is input processing via CLI parsing and output processing:

cats/cats/__init__.py

Lines 29 to 33 in 34c951e

def parse_arguments():
"""
Parse command line arguments
:return: [dict] parsed arguments
"""

and output processing via the CATSOutput class:

cats/cats/__init__.py

Lines 189 to 196 in 34c951e

class CATSOutput:
"""Carbon Aware Task Scheduler output"""
carbonIntensityNow: CarbonIntensityAverageEstimate
carbonIntensityOptimal: CarbonIntensityAverageEstimate
location: str
countryISO3: str
emmissionEstimate: Optional[Estimates] = None

all defined there, making the file over 300 lines long.

Unless there is a good reason these are defined there, we should move that logic out to appropriate modules, to keep the __init__.py lightweight module initialisation as per its intention.

Suggestion

I would personally move the CLI parsing out to a new module called cli.py and the CATSOutput and related logic to a new module output.py, or perhaps move all of it out to one new module called interface.py. Happy to take suggestions though, I am not tied to those as the solution.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants