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.

Sign up for GitHub

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

Jump to bottom

use constant for training data version everywhere #10909

Merged
merged 1 commit into from
Feb 22, 2022
Merged

use constant for training data version everywhere #10909

merged 1 commit into from
Feb 22, 2022

Conversation

indam23
Copy link
Contributor

@indam23 indam23 commented Feb 16, 2022 •
edited
Loading

Proposed changes:

  • Reference the constant LATEST_TRAINING_DATA_FORMAT_VERSION everywhere instead of hardcoding "3.0". This makes it simpler to bump the training data version in the future (will be required soon).

Status (please check what you already did):

  • updated some tests for the functionality
  • updated the documentation
  • updated the changelog (please check changelog for instructions)
  • reformat files using black (please check Readme for instructions)

@indam23 indam23 marked this pull request as ready for review February 16, 2022 23:18
@indam23 indam23 marked this pull request as draft February 17, 2022 00:28
@indam23 indam23 mentioned this pull request Feb 17, 2022
Merged
5 tasks
ancalita
ancalita reviewed Feb 17, 2022
View reviewed changes
Copy link
Member

@ancalita ancalita left a comment

Choose a reason for hiding this comment

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

Looks great 🎸 Only one tiny comment regarding the removal of quotation marks around the constant when used in yaml.

tests/cli/test_rasa_data.py Show resolved Hide resolved
@indam23 indam23 marked this pull request as ready for review February 21, 2022 09:17
@indam23
Copy link
Contributor Author

indam23 commented Feb 21, 2022

@ancalita all passing locally now, saw a failure in CI that I'm rerunning for because it looks out of date (?)

@indam23 indam23 force-pushed the constant_training_data_version branch from 02352e9 to 7a6f106 Compare February 21, 2022 10:39
@indam23 indam23 requested a review from a team as a code owner February 21, 2022 11:13
ancalita
ancalita approved these changes Feb 21, 2022
View reviewed changes
Copy link
Member

@ancalita ancalita left a comment

Choose a reason for hiding this comment

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

Happy to approve to unblock you 💯 Just a few more tests failing need your attention, gave a possible suggestion.

tests/core/test_migrate.py Show resolved Hide resolved
@indam23 indam23 enabled auto-merge (squash) February 22, 2022 09:50
@indam23 indam23 force-pushed the constant_training_data_version branch from b7de100 to 6362e17 Compare February 22, 2022 10:21
@indam23 indam23 merged commit 19439e1 into main Feb 22, 2022
@indam23 indam23 deleted the constant_training_data_version branch February 22, 2022 11:13
indam23 added a commit that referenced this pull request Jul 27, 2022
@indam23
use constant for training data version everywhere (#10909)
a7a0a73 
* rebase

* fix import of constant name

* fix quoting

* Always use DoubleQuotedScalarString for training data version

* Add another case of DoubleQuotedScalarString

* missing fstrings
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ancalita ancalita ancalita approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@indam23 @ancalita