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

Fix #4668: TextInput classifer Testing #4700

Merged
merged 1 commit into from
Feb 17, 2018

Conversation

akumar1503
Copy link
Contributor

Explanation

Prediction using TextInput classifier was producing console errors due to misspelled services.
There was error in prediction in the front end due to missing dependencies of TextInput classifier prediction.

Checklist

  • The PR title starts with "Fix #bugnum: ", followed by a short, clear summary of the changes.
  • The PR explanation includes the words "Fixes #bugnum: ...".
  • The linter/Karma presubmit checks have passed.
    • These should run automatically, but if not, you can manually trigger them locally using python scripts/pre_commit_linter.py and bash scripts/run_frontend_tests.sh.
  • The PR is made from a branch that's not called "develop".
  • The PR follows the style guide.
  • The PR is assigned to an appropriate reviewer.
    • If you're a new contributor, please ask on Gitter for someone to assign a reviewer.
    • If you're not sure who the appropriate reviewer is, please assign to the issue's "owner" -- see the "talk-to" label on the issue.

@codecov-io
Copy link

Codecov Report

Merging #4700 into develop will not change coverage.
The diff coverage is 0%.

Impacted file tree graph

@@           Coverage Diff            @@
##           develop    #4700   +/-   ##
========================================
  Coverage    44.72%   44.72%           
========================================
  Files          384      384           
  Lines        23405    23405           
  Branches      3791     3791           
========================================
  Hits         10467    10467           
  Misses       12938    12938
Impacted Files Coverage Δ
...es/exploration_editor/editor_tab/StateResponses.js 2.64% <ø> (ø) ⬆️
...pages/exploration_editor/editor_tab/StateEditor.js 50% <0%> (ø) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 9022b3d...33963e5. Read the comment docs.

@@ -24,7 +24,7 @@ class TextInput(base.BaseInteraction):
description = 'Allows learners to enter arbitrary text strings.'
display_mode = base.DISPLAY_MODE_INLINE
is_trainable = True
_dependency_ids = []
_dependency_ids = ['text_input_prediction']
Copy link
Contributor

@prasanna08 prasanna08 Feb 16, 2018

Choose a reason for hiding this comment

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

Seems like @anmolshkl missed this step (along with that HTML file which is necessary).

Copy link
Contributor

Choose a reason for hiding this comment

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

@GanitGenius so now prediction service is working fine without any errors, right?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yes, i didn't find any errors after making the changes.

Copy link
Contributor

@prasanna08 prasanna08 left a comment

Choose a reason for hiding this comment

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

LGTM.

@seanlip seanlip merged commit 0f4bb06 into oppia:develop Feb 17, 2018
@akumar1503 akumar1503 deleted the textInputClassifierTest branch February 17, 2018 09:19
@akumar1503
Copy link
Contributor Author

thanks.

code247 pushed a commit to code247/oppia that referenced this pull request Feb 24, 2018
code247 pushed a commit to code247/oppia that referenced this pull request Feb 24, 2018
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.

4 participants