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

Feature/default co2 rf method #49

Merged
merged 5 commits into from
Nov 25, 2024
Merged

Conversation

stefan-voelk
Copy link
Collaborator

@stefan-voelk stefan-voelk commented Nov 21, 2024

Before you get started

  • 🙋‍♀️ Create an issue to discuss what you are going to do!

Description

Please include a summary of the changes and the related issue. Please also include relevant motivation and context.

Note

  • This feature goes beyond defining a default CO2 RF method: Any other settings can be included in DEFAULT_CONFIG within module read_config. These default settings will be added to config if not specified by user in config file. Additionally, a logging message is output to inform user which default settings are used.
  • Added new dependency, python package "deepmerge" for merging nested dictionaries, e.g. config and added_dict.

Type of change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Documentation change

How has this been tested?

Please describe the software tests that you ran to verify your changes. Provide instructions so we can reproduce. Please also list any relevant details for your test configuration

  • Update of tests inTestCheckConfig
  • Add TestGetKeysValues
  • Add TestAddDefaultConfig

Test configuration:

  • Operating system: local machine: Windows 11 Enterprise (64-Bit)
  • Other relevant information:

Checklist

  • My code follows the style guidelines of this project
  • I have performed a self-review of my code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes
  • Any changed dependencies have been added or removed correctly

Any default configuration settings can be included within read_config DEFAULT_CONFIG. If setting not defined in config, it will be added from there.
Any default configuration settings can be included within read_config DEFAULT_CONFIG. If setting not defined in config, it will be added from there.
Update TestCheckConfig. Add TestGetKeysValues, TestAddDefaultConfig
@stefan-voelk stefan-voelk added the module: core Related to the core module label Nov 21, 2024
@stefan-voelk stefan-voelk added this to the v0.9.0 milestone Nov 21, 2024
Copy link
Collaborator

@liammegill liammegill left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good! Just a few minor comments. Whilst doing this review with a clean installation of OpenAirClim, I noticed that a number of the tests did not pass, which I now found out to be because they require create_test_files.py to have been run. I opened issue #50 to address this.

example/example.toml Outdated Show resolved Hide resolved
tests/read_config_test.py Outdated Show resolved Hide resolved
openairclim/read_config.py Outdated Show resolved Hide resolved
@liammegill liammegill linked an issue Nov 22, 2024 that may be closed by this pull request
Update of example.toml, CONFIG_TEMPLATE in read_config.py and tests/read_config_test.py
Update of openairclim.rst for automatic generation of API doc for module calc_cont.py
Copy link
Collaborator

@liammegill liammegill left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me!

@stefan-voelk stefan-voelk merged commit fd5e8b0 into dev Nov 25, 2024
1 check passed
@stefan-voelk stefan-voelk deleted the feature/default-co2-rf-method branch November 25, 2024 15:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
module: core Related to the core module
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Default CO2 RF method
2 participants