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

Add interactive configuration generation #409

Merged
merged 36 commits into from Mar 27, 2020

Conversation

desilinguist
Copy link
Member

@desilinguist desilinguist commented Mar 19, 2020

This PR closes #354 and #397.

It adds interactive configuration generation for all of the tools. Non-interactive or batch-mode configuration generation was previously added by #398.

To try out interactive generation, just run X generate --interactive where X is any of the five RSM command line tools. You can also add --subgroups for X = rsmtool, rsmeval, and rsmcompare.

The actual interaction bit is mostly handled by the excellent prompt_toolkit library. Here's what this this PR does specifically:

  • The configuration generation (both non-interactive and interactive) is handled by a class called rsmtool.utils.commandline.ConfigurationGenerator. The generate() method handles the non-interactive generation and the interact() method handles the interactive generation.

  • All of the required fields for the 5 tools and some of the more important optional fields are displayed to the user to provide inputs for them. The appropriate auto-completion and value validation are attached to each fields so that the user is shown appropriate options just like in interactive IPython sessions (which is where prompt_toolkit was spun out of). All of this is handled by the new class rsmtool.utils.commandline.InteractiveField. Essentially, when the interact() method is called, an InteractiveField instance is created for each of the chosen fields with the appropriate completer and validator attached and shown to the user. The values input by the user and then used to create a configuration dictionary and then eventually output as a string.

  • The chosen fields are contained in rsmtool.utils.constants.INTERACTIVE_MODE_METADATA which also defines the metadata for each field. This metadata is used to structure the interaction model (what to display, how to complete, how to validate etc.). The metadata consists of:

    • the label to be shown for the field in interactive mode,
    • the type of the field
    • whether the field takes a single value or multiple values (think "subgroups")
    • whether the field takes its value from a fixed list of choices
  • Testing for this specific functionality was quite tricky. We don't really want to test the prompt_toolkit functionality as that is already tested outside of our code. So, in keeping with the best practices of unit testing, we use mocking to mock the functionality provided by prompt_toolkit and only test that our code is doing the right thing. The new testing classes added here are test_utils.TestInteractiveField and test_utils.TestInteractiveGenerate. The first class checks that InteractiveField works as expected and the second checks that the ConfigurationGenerator.interact() works as expected. Between the two of them, we are covering the whole functionality.

  • This PR also adds documentation as follows: it adds a main page at the top level called "Auto-generating configuration files" that contains documentation for interactive, non-interactive generation (which was previously in each tool's configuration file page), and also API usage. I also added multiple screencasts that illustrate how the interactive generation works. Finally, I added links to this top-level page from the configuration file pages of all 5 tools and also all 5 tutorials. I updated the API documentation to include the documentation for the ConfigurationGenerator class and its generate() method and pointed to that from the top-level page too.

  • This PR also removes all imports from __init__.py other than the 5 main API functions. This meant minor changes to the API documentation.

@desilinguist desilinguist added this to In progress in RSMTool 8 via automation Mar 19, 2020
@pep8speaks
Copy link

pep8speaks commented Mar 19, 2020

Hello @desilinguist! Thanks for updating this PR.

Line 322:101: E501 line too long (119 > 100 characters)
Line 511:101: E501 line too long (128 > 100 characters)
Line 730:101: E501 line too long (115 > 100 characters)
Line 736:101: E501 line too long (108 > 100 characters)

Line 224:101: E501 line too long (103 > 100 characters)
Line 229:101: E501 line too long (111 > 100 characters)
Line 231:101: E501 line too long (103 > 100 characters)
Line 241:101: E501 line too long (149 > 100 characters)
Line 243:101: E501 line too long (108 > 100 characters)
Line 245:101: E501 line too long (116 > 100 characters)
Line 246:101: E501 line too long (102 > 100 characters)
Line 247:101: E501 line too long (101 > 100 characters)
Line 251:101: E501 line too long (101 > 100 characters)
Line 253:101: E501 line too long (101 > 100 characters)
Line 256:101: E501 line too long (104 > 100 characters)
Line 258:101: E501 line too long (101 > 100 characters)
Line 260:101: E501 line too long (105 > 100 characters)
Line 264:101: E501 line too long (101 > 100 characters)
Line 271:101: E501 line too long (105 > 100 characters)

Comment last updated at 2020-03-27 16:07:56 UTC

# Conflicts:
#	rsmtool/rsmcompare.py
#	rsmtool/rsmeval.py
#	rsmtool/rsmpredict.py
#	rsmtool/rsmsummarize.py
#	rsmtool/rsmtool.py
#	rsmtool/utils/commandline.py
#	tests/test_utils.py
@coveralls
Copy link

coveralls commented Mar 19, 2020

Coverage Status

Coverage increased (+0.07%) to 90.409% when pulling 97f6493 on interactive-config-generation into ddb0452 on master.

@aloukina
Copy link
Collaborator

When generating file for RSMTool, you get some of the optional fields (e.g. second human score, min, max). In the generated file though these are listed under // OPTIONAL: replace default values below based on your data. with some fields having indeed default values but other fields having the values the user had set in the interactive mode. Is there a way to regroup the fields to keep the unassigned fields together?

@aloukina
Copy link
Collaborator

rsmeval generate displays the following prompt: Name of column in evaluation data that contains human scores, if any: - this is not an optional field so it should be Name of column in evaluation data that contains human scores with sc1 as default, right?

@desilinguist
Copy link
Member Author

rsmeval generate displays the following prompt: Name of column in evaluation data that contains human scores, if any: - this is not an optional field so it should be Name of column in evaluation data that contains human scores with sc1 as default, right?

human_score_column is listed in the optional section for rsmeval. See here: https://github.com/EducationalTestingService/rsmtool/blob/master/rsmtool/utils/constants.py#L136

@aloukina
Copy link
Collaborator

That's because it defaults to sc1... Our definition of optional is not very transparent as I am realizing.

@desilinguist
Copy link
Member Author

Ah, I see. Yes "sc1" will be automatically in the config file so you are saying I just need to display "sc1" in the interactive prompt.

Copy link
Collaborator

@aloukina aloukina left a comment

Choose a reason for hiding this comment

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

Few minor things as I was looking at the code.

rsmtool/utils/constants.py Outdated Show resolved Hide resolved
rsmtool/utils/constants.py Outdated Show resolved Hide resolved
rsmtool/utils/constants.py Outdated Show resolved Hide resolved
rsmtool/utils/constants.py Outdated Show resolved Hide resolved
rsmtool/utils/constants.py Outdated Show resolved Hide resolved
@desilinguist desilinguist changed the title [WIP] Add interactive configuration generation Add interactive configuration generation Mar 24, 2020
@desilinguist
Copy link
Member Author

When generating file for RSMTool, you get some of the optional fields (e.g. second human score, min, max). In the generated file though these are listed under // OPTIONAL: replace default values below based on your data. with some fields having indeed default values but other fields having the values the user had set in the interactive mode. Is there a way to regroup the fields to keep the unassigned fields together?

This may not be possible because all of the optional fields are generated in sorted order right now and the comment is inserted before the first optional field. Doing this would require extra bookkeeping - perhaps we can just tweak the message in the comment?

@aloukina
Copy link
Collaborator

I agree that it's a nice to have - let's see how much people use the functionality.

@desilinguist desilinguist linked an issue Mar 24, 2020 that may be closed by this pull request
Copy link
Contributor

@mulhod mulhod left a comment

Choose a reason for hiding this comment

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

This looks great! By and large, my comments are superficial. I have a lot of suggestions for things like typos, etc.

doc/automated_configuration.rst Outdated Show resolved Hide resolved
doc/automated_configuration.rst Outdated Show resolved Hide resolved
doc/automated_configuration.rst Outdated Show resolved Hide resolved
doc/automated_configuration.rst Outdated Show resolved Hide resolved
doc/automated_configuration.rst Outdated Show resolved Hide resolved
rsmtool/utils/commandline.py Outdated Show resolved Hide resolved
rsmtool/utils/commandline.py Outdated Show resolved Hide resolved
rsmtool/utils/commandline.py Outdated Show resolved Hide resolved
rsmtool/utils/commandline.py Outdated Show resolved Hide resolved
rsmtool/utils/commandline.py Outdated Show resolved Hide resolved
RSMTool 8 automation moved this from In progress to Review in progress Mar 26, 2020
desilinguist and others added 4 commits March 26, 2020 19:49
Co-Authored-By: Matt Mulholland <mulhodm@gmail.com>
- Fix how field types are quoted in docstrings.
- Spell out input and intermediate file extensions in docstrings.
@desilinguist
Copy link
Member Author

desilinguist commented Mar 27, 2020

Thanks for the comprehensive review @mulhod! Much appreciated. I have addressed all of your comments.

RSMTool 8 automation moved this from Review in progress to Reviewer approved Mar 27, 2020
Copy link
Contributor

@mulhod mulhod 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! This is a really nice functionality. I think it will help a lot of people who might want to use RSMTool but may not know where to start.

Copy link
Collaborator

@aloukina aloukina 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 to me - very exciting!

doc/automated_configuration.rst Show resolved Hide resolved
doc/automated_configuration.rst Outdated Show resolved Hide resolved
Co-Authored-By: Anastassia Loukina <aloukina@users.noreply.github.com>
@desilinguist desilinguist merged commit 2faea99 into master Mar 27, 2020
RSMTool 8 automation moved this from Reviewer approved to Done Mar 27, 2020
@delete-merged-branch delete-merged-branch bot deleted the interactive-config-generation branch March 27, 2020 17:37
srhrshr pushed a commit to srhrshr/rsmtool that referenced this pull request Oct 23, 2021
…ingService/interactive-config-generation

Add interactive configuration generation
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
No open projects
RSMTool 8
  
Done
Development

Successfully merging this pull request may close these issues.

Trim imports in __init__.py Add config generator
5 participants