jiggzson / nerdamer

a symbolic math expression evaluator for javascript
http://www.nerdamer.com
MIT License
517 stars 82 forks source link

Add Unit Tests (for LaTeX conversion) #430

Closed tecosaur closed 5 years ago

tecosaur commented 5 years ago

As discussed in #426

Uses the Mocha testing framework

tecosaur commented 5 years ago

@Happypig375 regarding:

Square brackets indicate vectors, so they should be treated as such.

That's ... not quite true.

Square brackets can indeed represent vectors in some cases, such as: image However so can parenthesis: image Square brackets and parenthesis can also be used to represent sets: image Square brackets can also be used to represent values at which an expression is evaluated image And square brackets and parenthesis can be mixed up to make expressions more clear when brackets are nested: image And so on...

What makes them vectors isn't so much the type of bracket (with regards to square brackets and parenthesis) but the fact that they contain an ordered set of values.

As such one cannot assume vectors when one sees square brackets.

Happypig375 commented 5 years ago

Currently nerdamer sees all square brackets as vectors, as other notations are unsupported.

If all notations you mentioned are to be supported, then what should nerdamer interpret (1, 2) as? A set? A vector? A range? How do you differentiate the vector [2(2+3)] from the integer [2(2+3)]?

tecosaur commented 5 years ago

Correct me if I'm wrong but I didn't think nerdamer supported anything with sets, so would the following approach work?

  1. Get string enclosed by brackets/parenthesis
  2. See if there's a comma
  3. If there is, treat as vector
  4. Else, treat as number

This makes the assumption that we don't care about 1D vectors. I can't think of any situation where you would offhand, but let me know if you can.

Also if this idea has any merit, it can be implemented only on the LaTeX importing side, or on both. Your thoughts?

jiggzson commented 5 years ago

I didn't realize there was such a lively discussion going on :laughing:. I haven't yet had a chance to review the beginning of the thread but I did want to respond to 2 things.

  1. As @Happypig375 pointed out, square brackets strictly represent vectors at the moment. The only other way to create a vector is with the built-in vector function.
  2. Round brackets with comma separated values internally map to a Collection object. It's currently somewhat of a loose end. Sets aren't currently supported although I hope to in the near future.

Correct me if I'm wrong but I didn't think nerdamer supported anything with sets, so would the following approach work?

Get string enclosed by brackets/parenthesis See if there's a comma If there is, treat as vector Else, treat as number

I'm guessing that this excludes functions such as max. Or are we strictly speaking about stand-alone brackets? I really think you would save yourself a lot of work by at least re-using the existing tokenizer. Brackets come back as Arrays and have a type property attached, square, curly, or round. Just a suggestion.

Happypig375 commented 5 years ago

Fyi sets are proposed at #350

jiggzson commented 5 years ago

Can you let me know when this PR is ready? @tecosaur, if it's not too much trouble, can this and future PR's be into dev. I have some changes on that branch so I'd appreciate that. Or at the very least pull in those changes. I've learned through @Happypig375 that when you "Squash and Merge" the contributor data is lost so I won't be doing that anymore when merging into master.

tecosaur commented 5 years ago

@jiggzson The purpose of this PR is (a) to provide a test suite for the LaTeX import functionality, and (b) so that you know what I'm talking about when I say test this and that are currently passing, and that and this are failing.

I've recently discovered the jasmine tests that are here, so I'm going to convert the mocha tests to jasmine.

Would you like me to cancel this PR, and make another after I've rewritten the tests (to the dev branch this time)?

Conversation can continue in #426

jiggzson commented 5 years ago

Would you like me to cancel this PR, and make another after I've rewritten the tests (to the dev branch this time)?

@tecosaur, I think that using one test suite would be preferred so yes.

@jiggzson The purpose of this PR is (a) to provide a test suite for the LaTeX import functionality, and (b) so that you know what I'm talking about when I say test this and that are currently passing, and that and this are failing.

The reason why I ask is that there seems to be some discussion about the accuracy of certain tests. I haven't reviewed any of them but unless I hear otherwise I'll assume that we're all in agreement.

tecosaur commented 5 years ago

Ok then. Will do :)