Skip to content

Calc module changes: previewing and <formulaequationinput> - #512

Merged
pbaratta merged 3 commits into
masterfrom
peterb/formula-preview
Aug 12, 2013
Merged

Calc module changes: previewing and <formulaequationinput>#512
pbaratta merged 3 commits into
masterfrom
peterb/formula-preview

Conversation

@pbaratta

Copy link
Copy Markdown
Contributor

How to see it

Make a "Numerical Input" or "Advanced > Math Expression Input" problem in studio. By default, it should use the new input type.

Alteratively add a <formulaequationinput/> to a <numericalresponse> or <formularesponse> in some problem XML.

Also, if you have the content-demos course in your setup, checkout the peterb/formula-eq-input branch, and navigate to the 'Formula Equation Input' sequential.

Changes (in brief)

  • Change the way evaluator in calc.py works.
  • Add a method latex_preview to take a tree and give a latex string representing it.
  • Create an input type formulaequationinput with associated javascript that calls latex_preview on user input.
  • Change studio and other parts of the code base to reference the new input type.

Reviewers

  • @sarina
  • @nedbat
  • Please find someone from Studio team to review Studio changes
  • @auraz or someone from Kiev team, as well.

Comment thread common/lib/calc/calc.py Outdated

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

What is thing? If thing is a tree call it tree. Just think this can be a bit more descriptive variable name.

@sarina

sarina commented Jul 30, 2013

Copy link
Copy Markdown
Contributor

Did a VERY brief once-over. I think generally this is good but there'll be a bit to clean up. Also some unit tests are failing. Hopefully some others can take passes at this and I'll return and give it a more thorough review throughout this week.

Comment thread common/lib/calc/tests/test_calc.py Outdated

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Why do we need an except clause here? The exception is caught and turned into a test failure. Why not just let the exception fly and become an error result? Then you wouldn't need to pragma the except.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

It was to provide a better error message on failure, but you could say it has enough context already for people to understand what's going on.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Yeah, once you need to pragma the code, you should think a few times about why we have code that isn't run.

@auraz

auraz commented Aug 2, 2013

Copy link
Copy Markdown
Contributor

I would like to have working example in repository, please add example to content-demos repository (https://github.com/MITx/content-demos), so we can look at it "live"

Comment thread common/lib/calc/calc.py Outdated

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

why not (here and above):

varname = tokens[0][0]
if self.case_sensitive:
    varname = varname.lower()

@auraz

auraz commented Aug 2, 2013

Copy link
Copy Markdown
Contributor

@vaxXxa will review python part.

@auraz

auraz commented Aug 2, 2013

Copy link
Copy Markdown
Contributor

@polesye will look at front-end part on Monday.

Comment thread common/lib/calc/preview.py Outdated

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Instead of having the else clause you can just initialize suffix = "" before the if

@cahrens

cahrens commented Aug 2, 2013

Copy link
Copy Markdown

Take a look at the pep8/pylint violations and clean up things in code you have touched.
http://jenkins.edx.org:8080/job/edx-feature-branch-tests/9958/violations/?

@cahrens

cahrens commented Aug 2, 2013

Copy link
Copy Markdown

There is a broken icon that shows up briefly when I first render the problem in Studio or type a formula in the text field.

Also, as discussed with Peter, the error messaging/format needs work ("extCouldn′tparseformula:").

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Why (??)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Also these are suffixes not prefixes, right?

- Create a method called `parse_algebra`. It takes a string of math and returns with a `pyparsing.ParseResults` object representing it.
- `evaluator` takes this tree and applies the old "parse actions" to it to get the same number as it used to.
- Change calc's API: `evaluator` to use `case_sensitive` rather than `cs`
- Add most of the capability for latex rendering
Named `FormulaEquationInput` (name up for debate)

- Based off ChemEqnIn
- Add FormulaEquationInput in inputtypes.py
- Add a call to a skeleton method for a preview

javascript:

- Queue up some MathJax
- Put some ordering on the AJAX requests: add a parameter when the request was started, when it returns check that it isn't outdated before displaying the preview
- Tests

Note: we moved the `jsinput` tests and DISABLED them, because they were causing the tests to fail.
@sarina

sarina commented Aug 12, 2013

Copy link
Copy Markdown
Contributor

:shipit:

pbaratta pushed a commit that referenced this pull request Aug 12, 2013
Calc module changes: previewing and <formulaequationinput>

To see individual commits, see tag peterb/formula-preview/presquash
@pbaratta
pbaratta merged commit bfccfba into master Aug 12, 2013
@pbaratta
pbaratta deleted the peterb/formula-preview branch August 12, 2013 16:24
pbaratta added a commit that referenced this pull request Aug 12, 2013
chrisrossi pushed a commit to jazkarta/edx-platform that referenced this pull request Mar 31, 2014
Kelketek referenced this pull request in open-craft/openedx-platform Sep 17, 2015
Remove south table shortening call.
diegomillan pushed a commit to eduNEXT/edx-platform that referenced this pull request Sep 14, 2016
…01-squashed

Django 1.8 merge from upstream release 2015-12-01
Sujeet1379 pushed a commit to chandrudev/edx-platform that referenced this pull request Nov 17, 2022
Danyal-Faheem added a commit to Danyal-Faheem/edx-platform that referenced this pull request Sep 1, 2025
The identityserver class provided by Open edX was overriding our custom header definitions by initializing an empty header object

We now add our own custom auth headers (User-Agent) to the authentication APIs to comply with wikimedias user-agent policy
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.

9 participants