Remove physical constants and suffixes from calc.py

[jolyonb]

In formula problems, a number of physical constants have been predefined, as well as some suffixes. In my experience, these only lead to student confusion. Students expect an error to be raised when invalid input is given. So, when the answer to a given problem is 2*m and students forget the multiplication sign and instead enter 2m and are graded incorrect, this leads to student frustration, excess discussion forum posts, extra work for the instructor, etc. The problem is exacerbated in an exam situation where correctness is not shown until the end of the exam.

The physical constants have been defined to be particular numbers, and often lead to unintended consequences. Students expect that undefined variables will raise an error message, but the variables c, Q, T and k are accepted without warning. These variables are also particularly useless as defined, as they're specified in particular units, and cannot be used when working with different units. Best practice is definitely to sample variables appropriately when allowing for formula input.

This PR removes all extraneous physical constants and suffixes.

[openedx-webhooks]

Thanks for the pull request, @jolyonb! I've created OSPR-2187 to keep track of it in JIRA. JIRA is a place for product owners to prioritize feature reviews by the engineering development teams.

Feel free to add as much of the following information to the ticket:

• supporting documentation
• edx-code email threads
• timeline information ("this must be merged by XX date", and why that is)
• partner information ("this is a course on edx.org")
• any other information that can help Product understand the context for the PR

All technical communication about the code itself will still be done via the GitHub pull request interface. As a reminder, our process documentation is here.

[pdpinch]

@jolyonb this is failing on the case-sensitivity test. The value for e seems to be reverting to the constant instead of using the value we set for E in the test. What's the correct behavior?

from https://build.testeng.edx.org/job/edx-platform-python-unittests-pr/48020/testReport/junit/calc.calc.tests.test_calc/EvaluatorTest/test_variable_case_sensitivity/

         variables = {'E': 1.0}
>       self.assertEqual(calc.evaluator(variables, {}, "e"), 1.0)
E       AssertionError: 2.718281828459045 != 1.0

[jolyonb]

Huh. There's a bug in the underlying code that's been dormant for years! This one isn't our fault!

[nedbat]

How do we get confidence that others aren't using these constants and will be broken if we remove them?

Also, the tests have to pass. We can't merge to master with a failing test.

[nedbat]

This is still labeled as WIP. Ping me when you are ready for a review.

[jolyonb]

I don't know that other people aren't using them, and this may well break some things. I want to begin a discussion regarding them though. From my discussions with other MOOC creators in physics and math, these are universally reviled as creating frustrations for students and extra work for staff on the discussion boards.

[jefrench]

I build the math MOOCs at MIT, and I can say that I have never used the built in suffixes or prefixes ever in all of the 3000+ problems I've build over the years. I imagine these were built in specifically for the original course 6.002x. I imagine that it is so case specific, that it is not very valuable for most others.

[nedbat]

@sstack22 Can we get a product assessment about changing these details of calc's behavior?

[nedbat]

jenkins run python

[nedbat]

jenkins run quality

[sstack22]

@nedbat - @shamck and @scottrish are triaging incoming Educator OSPRS, adding them here.

[edx-status-bot]

Your PR has finished running tests.

[jolyonb]

jenkins run python

[edx-status-bot]

Your PR has finished running tests.

[jolyonb]

This line has to go. The code gives unpredictable results when there is a name collision due to case insensitivity. This is an old issue, and not one that anybody actually runs into. Also, we should not be testing for unpredictable results.

[nedbat]

@shamck Are you giving a "product thumbs-up" to this change?

[edx-status-bot]

Your PR has finished running tests.

[edx-status-bot]

Your PR has finished running tests.

[pdpinch]

jenkins run python

[edx-status-bot]

Your PR has finished running tests.

[edx-status-bot]

Your PR has finished running tests.

[pdpinch]

Squashed commits and removed the 'WIP' from the title.

[nedbat]

@sstack22 @scottrish @shamck I don't understand Product's opinion here.

[shamck]

Confirming that this has product approval and is ready to be assigned to a development team.

[miruto]

Please make this change!! Having these constants pre-defined is messing up all of my students in 8.02x right now!

[jolyonb]

@nedbat Can we get an update on this please?

[openedx-webhooks]

@jolyonb 🎉 Your pull request was merged!

Please take a moment to answer a two question survey so we can improve your experience in the future.

[pdpinch]

Woot!

[edx-pipeline-bot]

EdX Release Notice: This PR has been deployed to the staging environment in preparation for a release to production on Friday, April 27, 2018.

[edx-pipeline-bot]

EdX Release Notice: This PR has been deployed to the production environment.