Redesign Dashboard and Modularize Backend #6
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
adijo
reviewed
Aug 24, 2020
co2_tracker/co2_tracker.py
Outdated
| output_dir (str): Directory path to which the experiment artifacts are saved. Saved to current directory | ||
| by default. | ||
| save_to_file (bool): Indicates if the emission artifacts should be logged to a file | ||
| :param project_name: Project name for current experiment run. Default value of "default" |
There was a problem hiding this comment.
I prefer the Google style docstrings that I had earlier, the type information is pretty helpful.
co2_tracker/co2_tracker.py
Outdated
| if cloud.is_on_private_infra | ||
| else "" | ||
| ) | ||
| if cloud.is_on_private_infra: |
There was a problem hiding this comment.
This can be moved to a separate method, something like _prepare_persistence_data for example. This will make the stop() method shorter and more readable.
co2_tracker/emissions.py
Outdated
| df: pd.DataFrame = self._data_source.get_cloud_emissions_data() | ||
|
|
||
| # TODO: deal with missing data | ||
| print(cloud.provider, cloud.region) |
co2_tracker/emissions.py
Outdated
| return ( | ||
| _get_country_emissions_energy_mix( | ||
| energy, geo, energy_mix_data_path=config.private_infra_energy_mix_path, | ||
| class Emissions: |
There was a problem hiding this comment.
I think pure functions are better here instead of this class. Reference. There is no real state here, I think data_source can just be another argument
co2_tracker/emissions.py
Outdated
|
|
||
| country_energy_mix: Dict = energy_mix[geo.country_iso_code] | ||
|
|
||
| emissions_percentage: Dict[str, float] = {} |
co2_tracker/output.py
Outdated
| posting to Json Box, saving to a database, sending a slack message etc. | ||
| """ | ||
|
|
||
| @abstractmethod | ||
| def flush(self, data: CO2Data): | ||
| def out(self, data: CO2Data): |
There was a problem hiding this comment.
I think flush() (which is used with the context of emptying a buffer) or persist() is more descriptive
…meters, outputs and visualization
GitHub Pages Sphinx Documentation
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
No description provided.