mitodl / mitx-grading-library

The MITx Grading Library, a python grading library for edX
https://edge.edx.org/courses/course-v1:MITx+grading-library+examples/
BSD 3-Clause "New" or "Revised" License
14 stars 9 forks source link

AsciiMath Preprocessor Errors #169

Closed jolyonb closed 5 years ago

jolyonb commented 5 years ago

Similar to the conj issue #163, we also have math processor errors when the following are typed in our boxes:

There are likely others that I am unaware of; these are just the ones that I thought of off the top of my mind in testing. (ETA: These are actually all of the ones that we care about. There are 4 other entries that are defined using mover, but they're for thinks like overbraces that are unlikely to ever be used in this context.)

Something about our escaping is bad for these quantities too (ETA: Like conj, they are also movers!). Perhaps we should have a list of things that don't get escaped? (and include conj on that list)

ChristopherChudzicki commented 5 years ago

@jolyonb I forgot why we are wrapping functions and variables, so I had to go back through the issues. I found #144. Do you recall if this is the only reason we are wrapping variables and function calls things? We should really add a comment in the code about why we're wrapping them.

Perhaps we should have a list of things that don't get escaped? (and include conj on that list)

This sounds like a much better idea than what I did in #164. However, if we do not wrap conj calls at all, something like conj(x)'/x is going to look bad. Maybe that's sufficiently rare that we don't care. Alternatively, I think it should be straightforward to make wrapFuncCalls and wrapVariables check if the target is a lonely mover symbol.

Incidentally, the complete list of mover symbols is:

AM.symbols.filter(symbol => symbol.tag === 'mover' )
  .reduce((array, symbol) => {
    array.push(symbol.input)
    return array 
  }, [] )
// ["bar", "conj", "ddot", "dot", "hat", "obrace", "overarc",
// "overbrace", "overline", "overparen", "overset", "stackrel", "tilde", "vec"]
jolyonb commented 5 years ago

I think the problem only arises when we escape the mover symbols in isolation. When we escaped functions, we escaped {:head(args):}, and everything works. It's when we escape {:head:} or {:head:}( that we have problems. We just need to make sure that the list of movers don't get escaped as variables, I suspect. Note that conj(x)' is not a valid variable or function, so it doesn't matter if it looks bad. I think you might have meant conj'(x)... I have no idea how that would work. I think we should add to the documentation that there are a list of names that are reserved and should not be used due to asciimath, and include all those movers. Can you imagine a variable called hat'?

ChristopherChudzicki commented 5 years ago

Note that conj(x)' is not a valid variable or function so it doesn't matter if it looks bad

Good point 😄

I think we should add to the documentation that there are a list of names that are reserved and should not be used due to asciimath

Sure, that's probably easy to add where we document the preprocessor.

So:

  1. variable names need to be escaped so that x'/2 doesn't look terrible` (#144)
  2. function names need be escaped so that p(x)/2 doesn't look terrible (per your comment in #145)
  3. Except: conj and all other mover symbols should never be escaped. Including.

1 and 2 are already done; regarding 3: @jolyonb Do you agree that conj(x)/2 does NOT need to be escaped into {:conj(x):}/2?

jolyonb commented 5 years ago

No, I disagree: conj(x) needs to be escaped into {:conj(x):}. Note that this renders perfectly fine. It's only when {:conj:} is rendered that there's a problem.

ChristopherChudzicki commented 5 years ago

No, I disagree: conj(x) needs to be escaped into {:conj(x):}. Note that this renders perfectly fine. It's only when {:conj:} is rendered that there's a problem.

I mentioned the possibility of not escaping conj(x) because I believe our original motivation for escaping function calls was that p(x)/2 displays poorly. However, overline(x)/2 and other mover symbols display fine.

But I'm happy to escape conj(x), hat(x), and all completed function calls for consistency.

I should have some time to resolve this tomorrow.

jolyonb commented 5 years ago

Ah, ok, I get you. They're not represented as function calls any more, and so it all works out, in principle. If escaping completed function calls doesn't break anything, then I think we might as well continue to do so. It may help in some instances that we haven't tested...

Many thanks :-)