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

YAML based config #14

Merged
merged 19 commits into from
Oct 19, 2023
Merged

YAML based config #14

merged 19 commits into from
Oct 19, 2023

Conversation

avishniakov
Copy link
Contributor

@avishniakov avishniakov commented Oct 11, 2023

I reworked the template to use the YAML configuration instead of the Python file. This makes this template reusable for a broader count of use cases and makes it aligned with general ZenML configuration best practices.

  • Configuration for HP tuning and other uses is now passed as extra config via YAML
  • Search space objects support lists or ranges. For range a dictionary of {"range":{"start":1,"end":10,"step":2}} is used.
  • Artifacts folder and inner objects are no longer needed
  • New get_model_from_config step to reconstruct ClassifierMixin from string-based configurations

Depends on zenml-io/zenml#1876

@avishniakov avishniakov marked this pull request as ready for review October 11, 2023 16:42
@avishniakov avishniakov requested review from strickvl and fa9r October 11, 2023 16:42
@strickvl strickvl added enhancement New feature or request internal labels Oct 11, 2023
@fa9r fa9r requested review from bcdurak and removed request for fa9r October 11, 2023 17:35
@fa9r
Copy link
Contributor

fa9r commented Oct 11, 2023

Not really familiar with config best practices so I'm tagging @bcdurak instead who worked on this before I believe

@avishniakov
Copy link
Contributor Author

MacOS tests are broken, to be followed up here

.github/workflows/ci.yml Outdated Show resolved Hide resolved
@avishniakov avishniakov requested a review from strickvl October 13, 2023 12:12
Copy link
Contributor

Choose a reason for hiding this comment

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

Nothing to do with this, but while I can see that people might prefer this kind of config for simple configurations, but I find this a decent bit less clear than having a Python-based config.

Copy link
Contributor Author

@avishniakov avishniakov Oct 13, 2023

Choose a reason for hiding this comment

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

Yeah, I also prefer Python, but it is breaking caching logic currently and might not lead to a new pipeline version generated in ZenML on config update.

@avishniakov avishniakov merged commit 80aaa6c into main Oct 19, 2023
13 checks passed
@avishniakov avishniakov deleted the feature/OSS-2494-yaml-config branch November 10, 2023 10:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request internal
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants