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

Workaround for min and sec parsing problem #2961

Closed
wants to merge 2 commits into from

Conversation

costerwi
Copy link
Contributor

@costerwi costerwi commented Jun 3, 2023

The expression parser math.evaluate currently has some confusion between the unit min (minutes) and the function min (minimum). Similarly for sec (seconds) and the function sec (secant). Presently, the units are mistakenly recognized as functions in the parser and results in errors. This issue was raised in #2095 and in later discussion within #2771

It seems to me that function names should always be followed by left parenthesis "(" and the proper fix would be to have the parser look for that character to recognize any function. A fix like that should allow functions and units to coexist so that the user could even define new functions with names that match unit names without trouble. Currently, new functions like this will fail just like the built-in sec and min.

I'm not familiar enough with the code to implement a change like describe above so, for a temporary workaround, I've added a regular expression to immediately rename any min and sec units found in the parse expression.

@josdejong
Copy link
Owner

The naming conflicts between functions and units sec and min are indeed unfortunate. We've been thinking about these naming conflicts before.

The way that symbols are resolved is: look it up in the provided scope, then look it up in the math namespace, and lastly, try to look it up as a unit. To resolve the naming conflict like you propose, we need to know whether the symbol will be invoked as a function or not. In a case like min(...) this is possible. But since mathjs supports lambdas, you do not always know upfront how a symbol will be used. In the following example we cannot know whether we must resolve min as function or as unit:

up = true
f = up ? max : min
f(values)

Because of this ambiguity, we decided to not resolve symbols based on usage the context. As a user of the library you can alter the behavior though if you want: you simply chose one of the two by defining a variable, or you can write a preprocessor like you did if you want to utilize context information. But there is not a one-size-fits-all solution I think.

And another workaround: wrap the unit in the function, like unit('2 minute') - unit('2 min')

@costerwi costerwi closed this Jul 22, 2023
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.

2 participants