Skip to content

Deprecate training_data on subpredictors.#960

Merged
anoto-moniz merged 1 commit intomainfrom
feature/deprecate-training_data
Aug 26, 2024
Merged

Deprecate training_data on subpredictors.#960
anoto-moniz merged 1 commit intomainfrom
feature/deprecate-training_data

Conversation

@anoto-moniz
Copy link
Copy Markdown
Collaborator

@anoto-moniz anoto-moniz commented Aug 23, 2024

For quite a while, the backend has only supported training data supplied on graph predictors. To support the existing interface where it can be provided on many subpredictors, the backend hoists any training data it finds up to the root.

Deprecating the associated fields in the SDK is a step towards no longer needing to do that.

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

For quite a while, the backend has only supported training data supplied
on graph predictors. To support the existing interface where it can be
provided on many subpredictors, the backend hoists any training data it
finds up to the root.

Deprecating the associated fields in the SDK is a step towards no longer
needing to do that.
@anoto-moniz anoto-moniz marked this pull request as ready for review August 23, 2024 17:25
@anoto-moniz anoto-moniz requested a review from kroenlein August 23, 2024 17:25
Copy link
Copy Markdown
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

Does this have any consequences for e2es?

@anoto-moniz
Copy link
Copy Markdown
Collaborator Author

LGTM

Does this have any consequences for e2es?

Nothing major. Just ensuring we don't reference these fields anywhere, and ensuring we only add training data to the graph predictors.

@anoto-moniz anoto-moniz merged commit c4d3625 into main Aug 26, 2024
@anoto-moniz anoto-moniz deleted the feature/deprecate-training_data branch August 26, 2024 13:39
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.

2 participants