Skip to content

default tags in BaseForecaster; added some new tags#1013

Merged
fkiraly merged 11 commits intomainfrom
forecasting-default-tags
Jun 26, 2021
Merged

default tags in BaseForecaster; added some new tags#1013
fkiraly merged 11 commits intomainfrom
forecasting-default-tags

Conversation

@fkiraly
Copy link
Copy Markdown
Collaborator

@fkiraly fkiraly commented Jun 21, 2021

I've added some default tags in BaseForecaster - these are automatically inherited via _all_tags.

I've also added two new tags that I anticipate we will need:

  • "X-y-must-have-same-index" - do X and y need to have the same index?
  • "enforce-index-type" argument to check_X/check_y if needed - this has to be a tag now, see the problem in Refactor Prophet #1005 .

i've also added these new tags to the extension template.

This PR also:

  • corrects a minor bug: mis-spelled tags in the extension template
  • adds the new tags to the allowed tags in tests
  • removes the requirement for tags to be boolean

@fkiraly fkiraly requested a review from mloning as a code owner June 21, 2021 21:06
@fkiraly fkiraly requested a review from aiwalter June 21, 2021 21:06
@fkiraly fkiraly mentioned this pull request Jun 21, 2021
1 task
@fkiraly fkiraly mentioned this pull request Jun 22, 2021
Copy link
Copy Markdown
Contributor

@mloning mloning left a comment

Choose a reason for hiding this comment

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

As discussed, I think this can go in as an intermediate step if it is helpful in the overall refactoring. Afterwards, we need to revisit the tags (see #957).

@fkiraly
Copy link
Copy Markdown
Collaborator Author

fkiraly commented Jun 23, 2021

ok - will merge once the tests pass

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