Skip to content

Updated postfix_evaluation.py to support Unary operators#8787

Merged
cclauss merged 17 commits intoTheAlgorithms:masterfrom
arijitde92:master
Aug 23, 2023
Merged

Updated postfix_evaluation.py to support Unary operators#8787
cclauss merged 17 commits intoTheAlgorithms:masterfrom
arijitde92:master

Conversation

@arijitde92
Copy link
Contributor

@arijitde92 arijitde92 commented May 30, 2023 •
edited by cclauss
Loading

Fixes #8754
Fixes #8724

Describe your change:

Checklist:

  • I have read CONTRIBUTING.md.
  • This pull request is all my own work -- I have not plagiarized.
  • I know that pull requests will not be merged if they fail the automated tests.
  • This PR only changes one algorithm file. To ease review, please open separate PRs for separate algorithms.
  • All new Python files are placed inside an existing directory.
  • All filenames are in all lowercase characters with no spaces or dashes.
  • All functions and variable names follow Python naming conventions.
  • All function parameters and return values are annotated with Python type hints.
  • All functions have doctests that pass the automated testing.
  • All new algorithms include at least one URL that points to Wikipedia or another similar explanation.
  • If this pull request resolves one or more open issues then the commit message contains Fixes: #{$ISSUE_NO}.

Updated postfix_evaluation.py to support Unary operators and floating point numbers. Fixes #8754.
Also merged evaluate_postfix_notations.py and postfix_evaluation.py into postfix_evaluation.py which fixes #8724.

arijitde92 and others added 2 commits May 30, 2023 14:58
@arijitde92
Updated postfix_evaluation.py to support Unary operators and floating…
5bbcee5 
… point numbers Fixes #8754 and #8724

Also merged evaluate_postfix_notations.py and postfix_evaluation.py into postfix_evaluation.py

Signed-off-by: Arijit De <arijitde2050@gmail.com>
@pre-commit-ci
[pre-commit.ci] auto fixes from pre-commit.com hooks
b4565e0 
for more information, see https://pre-commit.ci
@algorithms-keeper algorithms-keeper bot added the tests are failing Do not merge until tests pass label May 30, 2023
@CaedenPH
Copy link
Contributor

CaedenPH commented May 30, 2023

@arijitde92 Put Fixes #8754 and #8724 in the pull request description

@arijitde92
Updated postfix_evaluation.py to support Unary operators and floating…
b985141 
… point numbers. Fixes #8754 and formatted code to pass ruff and black test.

Also merged evaluate_postfix_notations.py and postfix_evaluation.py into postfix_evaluation.py which fixes #8724 and made sure it passes doctest

Signed-off-by: Arijit De <arijitde2050@gmail.com>
@algorithms-keeper algorithms-keeper bot removed the tests are failing Do not merge until tests pass label May 30, 2023
@arijitde92
Updated postfix_evaluation.py to support Unary operators and floating…
800b07a 
… point numbers. Fixes #8754, #8724 and formatted code to pass ruff and black test.

Also merged evaluate_postfix_notations.py and postfix_evaluation.py into postfix_evaluation.py which fixes #8724 and made sure it passes doctest. Also updated type hinting which was required by pre-commit

Signed-off-by: Arijit De <arijitde2050@gmail.com>
@algorithms-keeper algorithms-keeper bot added the awaiting reviews This PR is ready to be reviewed label May 30, 2023
arijitde92 added 2 commits May 30, 2023 16:11
@arijitde92
Fixed return type hinting required by pre commit for evaluate function
70fd7fd 
Signed-off-by: Arijit De <arijitde2050@gmail.com>
@arijitde92
Changed line 186 to return only top of stack instead of calling the g…
ef175af 
…et_number function as it was converting float values to int, resulting in data loss. Fixes #8754 and #8724

Signed-off-by: Arijit De <arijitde2050@gmail.com>
@arijitde92
Copy link
Contributor Author

arijitde92 commented May 30, 2023

Hi @CaedenPH , @amirsoroush , Please review my PR and suggest changes if any. postfix_evaluation.py now works with unary operators and floating point numbers.
Plesee see example output below-

Enter a Postfix Expression (space separated): 2 1 + 3 *
Do you want to see stack contents while evaluating? [y/N]: 
Result =  9
Do you want to enter another expression? [y/N]: y
Enter a Postfix Expression (space separated): 4 13 5 / +
Do you want to see stack contents while evaluating? [y/N]: y
 Symbol  |    Action    | Stack
-----------------------------------
       4 | push(4)      | [4]
      13 | push(13)     | [4, 13]
       5 | push(5)      | [4, 13, 5]
         | pop(5)       | [4, 13]
         | pop(13)      | [4]
       / | push(13/5)   | [4, 2.6]
         | pop(2.6)     | [4]
         | pop(4)       | []
       + | push(4+2.6)  | [6.6]
Result =  6.6
Do you want to enter another expression? [y/N]: y
Enter a Postfix Expression (space separated): 2 - 3 +
Do you want to see stack contents while evaluating? [y/N]: y
 Symbol  |    Action    | Stack
----------------------------------
       2 | push(2)      | [2]
         | pop(2)       | [-2]
       - | push(-2)     | [-2]
       3 | push(3)      | [-2, 3]
         | pop(3)       | [-2]
         | pop(-2)      | []
       + | push(-2+3)   | [1]
Result =  1
Do you want to enter another expression? [y/N]: 

Process finished with exit code 0

@cclauss cclauss changed the title Updated postfix_evaluation.py to support Unary operators. Fixes #8754 and #8724 Updated postfix_evaluation.py to support Unary operators Jun 2, 2023
@cclauss
Copy link
Member

cclauss commented Jun 2, 2023

Seems like a lot more code to solve the same problem. Is it faster?

@umairefiroz

This comment was marked as off-topic.

Sign in to view
@arijitde92
Copy link
Contributor Author

arijitde92 commented Jun 2, 2023 •
edited
Loading

Seems like a lot more code to solve the same problem. Is it faster?

Hi @cclauss ,
I haved added extra functions and extra code to handle edge cases. For example-
get_number is used to simultaneously check whether the string is a number or not. In the current version it is checked by .isdigit() but floating point numbers cannot be detected by isdigit. The current postfix evaluation can only handle integer numbers.
is_operator is simply a helper function such that the program does'nt crash when it gets some other characters apart from numbers and mathematical operators (+,-,*,/,^) and prompts appropriate error message.
The evaluate() method is basically the solve() method of the current implementation with additional if else block to handle wrong input and unary operators. I have not changed the postfix evaluation logic.
is_yes is just a helper function as I have made the program a little bit more interactive in the main method by promtping for more expressions and asking choice for the user. If you want I can remove this part.

I am new to open source contribution. Please guide me on what else I can do to make the code shorter or faster.
Thanks.

@arijitde92
Copy link
Contributor Author

arijitde92 commented Jun 24, 2023

Hi @CaedenPH @cclauss @darkstar @amirsoroush, is there anything else I can do to improve the code? Could you please review the PR and let me know if any changes are required?

1 similar comment
@arijitde92
Copy link
Contributor Author

arijitde92 commented Jul 26, 2023

Hi @CaedenPH @cclauss @darkstar @amirsoroush, is there anything else I can do to improve the code? Could you please review the PR and let me know if any changes are required?

@arijitde92 arijitde92 mentioned this pull request Jul 29, 2023
Closed
tianyizheng02
tianyizheng02 requested changes Jul 31, 2023
View reviewed changes
Copy link
Contributor

@tianyizheng02 tianyizheng02 left a comment

Choose a reason for hiding this comment

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

General feedback: pay attention to where you can invert conditions or use early breaks/returns in order to simplify the control flow and avoid heavily nested code

@algorithms-keeper algorithms-keeper bot added the tests are failing Do not merge until tests pass label Aug 4, 2023
@arijitde92
Made the requested changes
d6874ad 
Also changed the code to make the evaluate function first convert all the numbers and then process the valid expression.
@algorithms-keeper algorithms-keeper bot removed the tests are failing Do not merge until tests pass label Aug 4, 2023
@arijitde92
Fixes #8754, #8724 Updated postfix_evaluation.py
b4521ea 
postfix_evaluation.py now supports Unary operators and floating point numbers.
Also merged evaluate_postfix_notations.py and postfix_evaluation.py into postfix_evaluation.py which fixes #8724. Added a doctest example with unary operator.
@arijitde92
Copy link
Contributor Author

arijitde92 commented Aug 4, 2023

Hi @tianyizheng02 , I have made the requested changes. Please check and let me know if it is okay. Thanks.

tianyizheng02
tianyizheng02 requested changes Aug 13, 2023
View reviewed changes
cclauss
cclauss reviewed Aug 17, 2023
View reviewed changes
tianyizheng02
tianyizheng02 requested changes Aug 17, 2023
View reviewed changes
@arijitde92
Fixes #8754, #8724 Updated postfix_evaluation.py
b4f51a6 
postfix_evaluation.py now supports Unary operators and floating point numbers.
Also merged evaluate_postfix_notations.py and postfix_evaluation.py into postfix_evaluation.py which fixes #8724. Added a doctest example with unary operator.
tianyizheng02
tianyizheng02 requested changes Aug 23, 2023
View reviewed changes
Copy link
Contributor

@tianyizheng02 tianyizheng02 left a comment

Choose a reason for hiding this comment

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

Some minor tweaks, but otherwise LGTM

data_structures/stacks/postfix_evaluation.py Outdated

Parameters
----------
token : str
Copy link
Contributor

@tianyizheng02 tianyizheng02 Aug 23, 2023

Choose a reason for hiding this comment

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

Suggested change
token : str
token : str or float

Copy link
Member

@cclauss cclauss Aug 23, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

Would it be str | float instead of or?

Why would we want token to be a float?

Copy link
Contributor

@tianyizheng02 tianyizheng02 Aug 23, 2023

Choose a reason for hiding this comment

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

Each element in the token list valid_expression is passed into this function to check if it's an operator—those elements may be numbers.

@arijitde92
Fixes #8754, #8724 Updated the parse_token function of postfix_evalua…
9a15148 
…tion.py

ostfix_evaluation.py now supports Unary operators and floating point numbers.
Also merged evaluate_postfix_notations.py and postfix_evaluation.py into postfix_evaluation.py which fixes #8724. Added a doctest example with unary operator and invalid expression.
tianyizheng02
tianyizheng02 previously requested changes Aug 23, 2023
View reviewed changes
data_structures/stacks/postfix_evaluation.py Outdated
Comment on lines 34 to 42
def parse_token(token: str | float) -> float | str:
"""
Converts the given data to appropriate number if it is indeed a number, else returns
the data as it is with a False flag. This function also serves as a check of whether
the input is a number or not.

Parameters
----------
token : str or float
Copy link
Contributor

@tianyizheng02 tianyizheng02 Aug 23, 2023

Choose a reason for hiding this comment

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

Suggested change
def parse_token(token: str | float) -> float | str:
"""
Converts the given data to appropriate number if it is indeed a number, else returns
the data as it is with a False flag. This function also serves as a check of whether
the input is a number or not.
Parameters
----------
token : str or float
def parse_token(token: str) -> float | str:
"""
Converts the given data to appropriate number if it is indeed a number, else returns
the data as it is with a False flag. This function also serves as a check of whether
the input is a number or not.
Parameters
----------
token : str

I meant for this suggestion to be for is_operator, not this function. I believe this function is only used when initially parsing the input list, so all tokens passed into this function should be strings.

Copy link
Contributor Author

@arijitde92 arijitde92 Aug 23, 2023

Choose a reason for hiding this comment

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

Yes understood. I have made the necessary changes.

data_structures/stacks/postfix_evaluation.py Outdated

Parameters
----------
token : str
Copy link
Contributor

@tianyizheng02 tianyizheng02 Aug 23, 2023

Choose a reason for hiding this comment

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

Each element in the token list valid_expression is passed into this function to check if it's an operator—those elements may be numbers.

@arijitde92
Fixes #8754, #8724 Updated postfix_evaluation.py
c11b18f 
postfix_evaluation.py now supports Unary operators and floating point numbers.
Also merged evaluate_postfix_notations.py and postfix_evaluation.py into postfix_evaluation.py which fixes #8724. Added a doctest example with unary operator and invalid expression.
cclauss
cclauss reviewed Aug 23, 2023
View reviewed changes
data_structures/stacks/postfix_evaluation.py
Function that evaluates postfix expression using a stack.
>>> evaluate(["2", "1", "+", "3", "*"])
9.0
>>> evaluate(["4", "13", "5", "/", "+"])
Copy link
Member

@cclauss cclauss Aug 23, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

Please add doctests for:

  • evaluate(["0"])
  • evaluate(["-0"])
  • evaluate(["1"])
  • evaluate(["-1"])
  • evaluate(["-1.1"])
  • evaluate(["2", "1.9", "+", "3", "*"])
  • evaluate(["2", "-1.9", "+", "3", "*"])
  • evaluate(["2", "--1.9", "+", "3", "*"])

@algorithms-keeper algorithms-keeper bot added the tests are failing Do not merge until tests pass label Aug 23, 2023
@algorithms-keeper algorithms-keeper bot added tests are failing Do not merge until tests pass and removed tests are failing Do not merge until tests pass labels Aug 23, 2023
@algorithms-keeper algorithms-keeper bot removed the tests are failing Do not merge until tests pass label Aug 23, 2023
@cclauss cclauss enabled auto-merge (squash) August 23, 2023 12:36
@cclauss cclauss dismissed tianyizheng02’s stale review August 23, 2023 12:36

Several code fixes

@cclauss cclauss merged commit 0a94380 into TheAlgorithms:master Aug 23, 2023
@algorithms-keeper algorithms-keeper bot removed the awaiting reviews This PR is ready to be reviewed label Aug 23, 2023
@isidroas isidroas mentioned this pull request Jan 25, 2025
Closed
14 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@cclauss cclauss cclauss approved these changes

@tianyizheng02 tianyizheng02 tianyizheng02 left review comments

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Need to support for unary operator in postfix evaluation. Duplicate solutions for postfix notation evaluation.

5 participants

@arijitde92 @CaedenPH @cclauss @umairefiroz @tianyizheng02