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

Error handling for get_tag #1450

Merged
merged 17 commits into from Oct 1, 2021
Merged

Error handling for get_tag #1450

merged 17 commits into from Oct 1, 2021

Conversation

fkiraly
Copy link
Collaborator

@fkiraly fkiraly commented Sep 24, 2021

This adds the possibility to raise an error when get_tag is used.

An error will always be raised if the function is used with parameter defaults and the tag with the specified tag_name does not exist, but this can be overridden by setting the new raise_error parameter to False.

This should hopefully prevent the human error that caused the bug in #1449 for the future.

FYI @aiwalter, @thayeylolu, @mloning

This PR also contains a second test for get_tag to cover the behaviour if raise_error=True, the old test covers the behaviour if raise_errpr=False with minor changes.

The change has caught the following bugs which are also fixed:

  • "enforce_index_type" tag was sometimes written "enforce-index-type", this was changed to "enforce_index_type" consistently
  • transformer base class did not have any default tag values set, which caused framework to look for non-existent tags - added a set of defaults
  • "coerce-X-to-pandas" was called in some classifiers but not set in the tine series classification base class

@fkiraly
Copy link
Collaborator Author

fkiraly commented Sep 24, 2021

@aiwalter, do you understand why this is not failling the tests you fixed in #1449? I'm surprised that it doesn't.

@fkiraly
Copy link
Collaborator Author

fkiraly commented Sep 24, 2021

found it, forgot the raise in front of ValueError.

aiwalter
aiwalter previously approved these changes Sep 30, 2021
sktime/transformations/base.py Show resolved Hide resolved
@fkiraly fkiraly merged commit f869fa4 into main Oct 1, 2021
@fkiraly fkiraly deleted the tag-error branch October 1, 2021 17:52
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.

None yet

2 participants