Skip to content

Conversation

@sfriedowitz
Copy link
Contributor

@sfriedowitz sfriedowitz commented Aug 31, 2022

Citrine Python PR

Adds support to configure estimator types for AutoML.

PR Type:

  • Breaking change (fix or feature that would cause existing functionality to change)
  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Maintenance (non-breaking change to assist developers)

Adherence to team decisions

  • I have added tests for 100% coverage
  • I have written Numpy-style docstrings for every method and class.
  • I have communicated the downstream consequences of the PR to others.
  • I have bumped the version in __version__.py

# We never exposed saving a model without training, so we should
# continue to do it automatically.
return self._train(predictor.uid)
# If the initial response is invalid, just return it
Copy link
Contributor Author

@sfriedowitz sfriedowitz Aug 31, 2022

Choose a reason for hiding this comment

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

@anoto-moniz We will never actually hit the .failed() branch until the API bug fix is in, but the logic is correct and tested for now.

@sfriedowitz sfriedowitz marked this pull request as ready for review August 31, 2022 22:12
Copy link
Collaborator

@kroenlein kroenlein left a comment

Choose a reason for hiding this comment

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

I didn't see a test verifying that an arbitrary string is not a valid AutoMLEstimator. Intentional?

@sfriedowitz sfriedowitz requested a review from kroenlein August 31, 2022 23:56
Copy link
Collaborator

@kroenlein kroenlein left a comment

Choose a reason for hiding this comment

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

LGTM. Get an e2e in there when you get a chance.

@sfriedowitz sfriedowitz merged commit 11571a3 into main Sep 1, 2022
@sfriedowitz sfriedowitz deleted the PLA-10206/user-configure-auto-ml branch September 1, 2022 15:28
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