Skip to content
This repository has been archived by the owner on Nov 3, 2023. It is now read-only.

Add taskmaster2 command-line arg to specify subset of domains #3135

Merged
merged 2 commits into from
Sep 30, 2020

Conversation

moyapchen
Copy link
Contributor

Patch description
Title

Testing steps
Before this change, parlai dd -t taskmaster2 --display-verbose displayed a bunch of sports conversations.

After this change, running parlai dd -t taskmaster2 --display-verbose --domains music displays music conversations. Tried on a few other domains to validate; also had a print in the _load_data() function.

Also verified that no argument case that all domains were used +
defining domains multiple times only used the last one.

Before this change, `parlai dd -t taskmaster2 --display-verbose` displayed a bunch of sports conversations.

After this change, running `parlai dd -t taskmaster2 --display-verbose --domains music` displays music conversations. Tried on a few other domains to validate; also had a print in the `_load_data()` function.

Also verified that no argument case that all domains were used +
defining `domains` multiple times only used the last one.
@github-actions
Copy link

Your PR contains a change to a task. Please paste the results of the following command into a comment:

python tests/datatests/test_new_tasks.py

@moyapchen
Copy link
Contributor Author

moyapchen commented Sep 30, 2020

python tests/datatests/test_new_tasks.py is broken on master with taskmaster2, so skipping. (Might look into why it's broken later)

Before this change, `parlai dd -t taskmaster2 --display-verbose`
displayed a bunch of sports conversations.

After this change, running `parlai dd -t taskmaster2
--display-verbose --domains music` displays a
music.

Also verified that no argument case that all domains were used.
Copy link
Contributor

@stephenroller stephenroller left a comment

Choose a reason for hiding this comment

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

does choices magically work with nargs='+'?

@moyapchen
Copy link
Contributor Author

Yeah, it yells at me if I spell things wrong.

@moyapchen moyapchen merged commit f315211 into master Sep 30, 2020
@moyapchen moyapchen deleted the domain_subset branch September 30, 2020 17:58
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants