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

Cleanup: parameters: extra=forbid, optimize: battery, inverter optional #361

Open
wants to merge 2 commits into
base: feature/config-overhaul
Choose a base branch
from

Conversation

Lasall
Copy link
Collaborator

@Lasall Lasall commented Jan 9, 2025

  • Don't allow extra fields for parameters (at least for now while changing API).
  • Allow both battery and inverter to be set optionally (atm optional battery not implemented, no API constraints).
  • inverter: Remove default max_power_wh
  • single_test_optimization: Add more cli-parameters
  • Docker: Fix entrypoint

Comment on lines 38 to 41
class ParametersBaseModel(BaseModel):
model_config = ConfigDict(extra="forbid")


Copy link
Contributor

@b0661 b0661 Jan 10, 2025

Choose a reason for hiding this comment

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

If this only holds for optimization parameters, please move to optimization/optimizationabc.py and call it OptimizationParametersBase. If only for devices do likewise. Why do you use BaseModel? The custom PydanticBaseModel supports several custom types that are not supported by BaseModel.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

It's basically for all REST-API related interfaces (e.g. parameters), I'm open for a better name.

@Lasall Lasall force-pushed the dl_dev-little-fixes branch 2 times, most recently from 270fabf to 4bcfbcc Compare January 10, 2025 17:59
 * Don't allow extra fields for parameters/REST-API (at least for now while
   changing API).
 * Allow both battery and inverter to be set optionally (atm optional
   battery not implemented, no API constraints).
 * inverter: Remove default max_power_wh
 * single_test_optimization: Add more cli-parameters
@Lasall Lasall force-pushed the dl_dev-little-fixes branch from 2db6f94 to 7609e7f Compare January 13, 2025 12:00
 * Secrets are not available anyway for forks.
@Lasall Lasall force-pushed the dl_dev-little-fixes branch from 184ca49 to 25d5e01 Compare January 13, 2025 12:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants