Fix brackets display #112

jianan1104 · 2022-11-15T17:43:46Z

Overview

Fix brackets display, replace \left( to \mathopen{}\left( and \right) to \mathclose{}\right)

Details

Modify function def _wrap_operand in the src/latexify/function_codegen.py line by 422, Change the original brackets string to fix the display.

before
after

References

#90

…ght) to \mathclose{}\right) Fix brackets display

odashi · 2022-11-15T17:46:11Z

@jianan1104 Thanks! You also need to fix all tests that are related to this change. Running ./checks.sh could check if the test passes or not.

odashi

I think there are other brackets in function_codegen.py, and it would be great if we fixed all of them.

jianan1104 · 2022-11-15T17:53:32Z

@jianan1104 Thanks! You also need to fix all tests that are related to this change. Running ./checks.sh could check if the test passes or not.

Thank you for reviewing. No problem. I'll fix it asap.

replace all \left( to \mathopen{}\left( and \rght) to \mathclose{}\right)

…ht) to \mathclose{}\right)

…d \right) to \mathclose{}\right)

jianan1104 · 2022-11-16T06:11:44Z

@odashi Hi, odashi. just fix and test the code, please check it.

odashi

The logic looks fine, thanks!

It looks some lines exceed the length limit (88 characters) though I didn't realize why Black didn't warn about them. Could you fix them as well?

odashi · 2022-11-16T06:53:28Z

checks.sh

@@ -3,4 +3,4 @@ set -eoux pipefail

 python -m pytest src -vv
 python -m black --check src
-python -m pflake8 src
+python -m flake8 src


Revert this change.

Okay. sorry for this change. I have no experience with pflake8(flake8 for pyproject), so I just edit it by intuition.

odashi · 2022-11-16T07:07:49Z

src/integration_tests/regression_test.py

 import math
+from collections.abc import Callable


There are some changes about only the import order, which are not related to the essential point of this pull request. Could you revert these changes to prevent breaking the blame history?

(Btw, I think I need to introduce some static checker (e.g., isort) to constrain the style around imports.)

No problem.

odashi · 2022-11-16T07:16:20Z

It looks the CI accidentally passes though it detected some errors:

Run python -m pflake8 src
src/integration_tests/regression_test.py:91:89: E501 line too long ([11](https://github.com/google/latexify_py/actions/runs/3476330689/jobs/5811657602#step:5:12)4 > 88 characters)
src/integration_tests/regression_test.py:99:89: E501 line too long (115 > 88 characters)
src/integration_tests/regression_test.py:227:89: E501 line too long (132 > 88 characters)
src/latexify/__init__.py:7:1: E722 do not use bare 'except'
src/latexify/codegen/function_codegen_test.py:59:89: E501 line too long (107 > 88 characters)
src/latexify/codegen/function_codegen_test.py:75:89: E501 line too long (108 > 88 characters)
src/latexify/codegen/function_codegen_test.py:104:89: E501 line too long (107 > 88 characters)
src/latexify/codegen/function_codegen_test.py:[12](https://github.com/google/latexify_py/actions/runs/3476330689/jobs/5811657602#step:5:13)0:89: E501 line too long (108 > 88 characters)
src/latexify/codegen/function_codegen_test.py:[14](https://github.com/google/latexify_py/actions/runs/3476330689/jobs/5811657602#step:5:15)5:89: E501 line too long (103 > 88 characters)
src/latexify/codegen/function_codegen_test.py:149:89: E501 line too long (105 > 88 characters)
src/latexify/codegen/function_codegen_test.py:[15](https://github.com/google/latexify_py/actions/runs/3476330689/jobs/5811657602#step:5:16)3:89: E501 line too long (115 > 88 characters)
src/latexify/codegen/function_codegen_test.py:[17](https://github.com/google/latexify_py/actions/runs/3476330689/jobs/5811657602#step:5:18)7:89: E501 line too long (125 > 88 characters)
src/latexify/codegen/function_codegen_test.py:[19](https://github.com/google/latexify_py/actions/runs/3476330689/jobs/5811657602#step:5:20)4:89: E501 line too long (91 > 88 characters)
src/latexify/codegen/function_codegen_test.py:198:89: E501 line too long (109 > 88 characters)
src/latexify/codegen/function_codegen_test.py:[20](https://github.com/google/latexify_py/actions/runs/3476330689/jobs/5811657602#step:5:21)3:89: E501 line too long (93 > 88 characters)
src/latexify/codegen/function_codegen_test.py:[22](https://github.com/google/latexify_py/actions/runs/3476330689/jobs/5811657602#step:5:23)3:89: E501 line too long (119 > 88 characters)
src/latexify/codegen/function_codegen_test.py:2[28](https://github.com/google/latexify_py/actions/runs/3476330689/jobs/5811657602#step:5:29):89: E501 line too long (114 > 88 characters)
src/latexify/codegen/function_codegen_test.py:229:89: E501 line too long (111 > 88 characters)

jianan1104 · 2022-11-16T07:37:01Z

It looks the CI accidentally passes though it detected some errors:

Run python -m pflake8 src
src/integration_tests/regression_test.py:91:89: E501 line too long ([11](https://github.com/google/latexify_py/actions/runs/3476330689/jobs/5811657602#step:5:12)4 > 88 characters)
src/integration_tests/regression_test.py:99:89: E501 line too long (115 > 88 characters)
src/integration_tests/regression_test.py:227:89: E501 line too long (132 > 88 characters)
src/latexify/__init__.py:7:1: E722 do not use bare 'except'
src/latexify/codegen/function_codegen_test.py:59:89: E501 line too long (107 > 88 characters)
src/latexify/codegen/function_codegen_test.py:75:89: E501 line too long (108 > 88 characters)
src/latexify/codegen/function_codegen_test.py:104:89: E501 line too long (107 > 88 characters)
src/latexify/codegen/function_codegen_test.py:[12](https://github.com/google/latexify_py/actions/runs/3476330689/jobs/5811657602#step:5:13)0:89: E501 line too long (108 > 88 characters)
src/latexify/codegen/function_codegen_test.py:[14](https://github.com/google/latexify_py/actions/runs/3476330689/jobs/5811657602#step:5:15)5:89: E501 line too long (103 > 88 characters)
src/latexify/codegen/function_codegen_test.py:149:89: E501 line too long (105 > 88 characters)
src/latexify/codegen/function_codegen_test.py:[15](https://github.com/google/latexify_py/actions/runs/3476330689/jobs/5811657602#step:5:16)3:89: E501 line too long (115 > 88 characters)
src/latexify/codegen/function_codegen_test.py:[17](https://github.com/google/latexify_py/actions/runs/3476330689/jobs/5811657602#step:5:18)7:89: E501 line too long (125 > 88 characters)
src/latexify/codegen/function_codegen_test.py:[19](https://github.com/google/latexify_py/actions/runs/3476330689/jobs/5811657602#step:5:20)4:89: E501 line too long (91 > 88 characters)
src/latexify/codegen/function_codegen_test.py:198:89: E501 line too long (109 > 88 characters)
src/latexify/codegen/function_codegen_test.py:[20](https://github.com/google/latexify_py/actions/runs/3476330689/jobs/5811657602#step:5:21)3:89: E501 line too long (93 > 88 characters)
src/latexify/codegen/function_codegen_test.py:[22](https://github.com/google/latexify_py/actions/runs/3476330689/jobs/5811657602#step:5:23)3:89: E501 line too long (119 > 88 characters)
src/latexify/codegen/function_codegen_test.py:2[28](https://github.com/google/latexify_py/actions/runs/3476330689/jobs/5811657602#step:5:29):89: E501 line too long (114 > 88 characters)
src/latexify/codegen/function_codegen_test.py:229:89: E501 line too long (111 > 88 characters)

I do not know if any lint tool can fix automatically? or just edit line by line?

odashi · 2022-11-16T07:59:59Z

I do not know if any lint tool can fix automatically? or just edit line by line?

Length limit error is usually resolved by running Black, but I guess long string literals couldn't be handled correctly. So (unfortunately) some errors need to be fixed manually.

Btw, I also noticed that the list above contains some existing errors in main. I will fix it first ASAP.

It looks the accident around flake8 was reported recently:
csachs/pyproject-flake8#19

odashi · 2022-11-16T08:13:54Z

@jianan1104 I fixed CIs. Could you merge the main into this pull request so that the flake8 errors kill the CI appropriately?

jianan1104 · 2022-11-16T11:54:15Z

@jianan1104 I fixed CIs. Could you merge the main into this pull request so that the flake8 errors kill the CI appropriately?

Sure. merged. wait for ci result...

jianan1104 · 2022-11-16T11:54:51Z

I do not know if any lint tool can fix automatically? or just edit line by line?

Length limit error is usually resolved by running Black, but I guess long string literals couldn't be handled correctly. So (unfortunately) some errors need to be fixed manually.

Btw, I also noticed that the list above contains some existing errors in main. I will fix it first ASAP.

It looks the accident around flake8 was reported recently: csachs/pyproject-flake8#19

@odashi Done

Fix CI error, and update to main.

This reverts commit cbdfe33.

This reverts commit 9f037ba.

pass all test

odashi

Really great work, thanks!

odashi · 2022-11-16T22:19:58Z

Fixes #90

jianan1104 · 2022-11-17T01:54:05Z

Really great work, thanks!

Glad to be a contributor.

fix(function_codegen.py): replace \left( to \mathopen{}\left( and \ri…

bd3c52c

…ght) to \mathclose{}\right) Fix brackets display

jianan1104 requested a review from odashi as a code owner November 15, 2022 17:43

odashi reviewed Nov 15, 2022

View reviewed changes

odashi added this to the v0.3 milestone Nov 15, 2022

jianan1104 added 4 commits November 15, 2022 18:05

fix(function_codegen.py): Fix brackets display

8747b1c

replace all \left( to \mathopen{}\left( and \rght) to \mathclose{}\right)

refactor(function_codegen.py): Remove '{}' for non-fstring

3f70423

fix(regression_test.py): Replace \left( to \mathopen{}\left( and \rig…

e13a283

…ht) to \mathclose{}\right)

fix(function_codegen_test.py): Replace \left( to \mathopen{}\left( an…

66cf5ee

…d \right) to \mathclose{}\right)

jianan1104 requested a review from odashi November 16, 2022 03:56

jianan1104 added 3 commits November 16, 2022 04:01

fix(checks.sh): Fix typo [pflake -> flake8]

cbdfe33

refactor: Using isort to sort imports alphabetically

9f037ba

refactor: Using black to reformat codes

7f1373b

odashi reviewed Nov 16, 2022

View reviewed changes

jianan1104 added 5 commits November 16, 2022 19:57

Merge pull request #1 from jianan1104/main

8558f11

Fix CI error, and update to main.

Revert "fix(checks.sh): Fix typo [pflake -> flake8]"

a2d1cd7

This reverts commit cbdfe33.

Revert "refactor: Using isort to sort imports alphabetically"

3dfefeb

This reverts commit 9f037ba.

refactor(regression_test.py): Lint codes to fix E501 line too long

cde8fa5

refactor(function_codegen_test.py): Lint codes to fix E501 line too long

da4e42c

pass all test

jianan1104 requested a review from odashi November 16, 2022 14:16

odashi approved these changes Nov 16, 2022

View reviewed changes

odashi added the feature label Nov 16, 2022

odashi merged commit 0a7eafa into google:main Nov 16, 2022

jianan1104 deleted the add-mathopen-and-mathclose-to-parentheses branch November 21, 2022 04:09

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix brackets display #112

Fix brackets display #112

jianan1104 commented Nov 15, 2022

odashi commented Nov 15, 2022 •

edited

Loading

odashi left a comment

jianan1104 commented Nov 15, 2022

jianan1104 commented Nov 16, 2022

odashi left a comment •

edited

Loading

odashi Nov 16, 2022

jianan1104 Nov 16, 2022

jianan1104 Nov 16, 2022

odashi Nov 16, 2022

jianan1104 Nov 16, 2022

jianan1104 Nov 16, 2022

odashi commented Nov 16, 2022

jianan1104 commented Nov 16, 2022

odashi commented Nov 16, 2022

odashi commented Nov 16, 2022 •

edited

Loading

jianan1104 commented Nov 16, 2022 •

edited

Loading

jianan1104 commented Nov 16, 2022 •

edited

Loading

odashi left a comment

odashi commented Nov 16, 2022

jianan1104 commented Nov 17, 2022

Fix brackets display #112

Fix brackets display #112

Conversation

jianan1104 commented Nov 15, 2022

Overview

Details

References

odashi commented Nov 15, 2022 • edited Loading

odashi left a comment

Choose a reason for hiding this comment

jianan1104 commented Nov 15, 2022

jianan1104 commented Nov 16, 2022

odashi left a comment • edited Loading

Choose a reason for hiding this comment

odashi Nov 16, 2022

Choose a reason for hiding this comment

jianan1104 Nov 16, 2022

Choose a reason for hiding this comment

jianan1104 Nov 16, 2022

Choose a reason for hiding this comment

odashi Nov 16, 2022

Choose a reason for hiding this comment

jianan1104 Nov 16, 2022

Choose a reason for hiding this comment

jianan1104 Nov 16, 2022

Choose a reason for hiding this comment

odashi commented Nov 16, 2022

jianan1104 commented Nov 16, 2022

odashi commented Nov 16, 2022

odashi commented Nov 16, 2022 • edited Loading

jianan1104 commented Nov 16, 2022 • edited Loading

jianan1104 commented Nov 16, 2022 • edited Loading

odashi left a comment

Choose a reason for hiding this comment

odashi commented Nov 16, 2022

jianan1104 commented Nov 17, 2022

odashi commented Nov 15, 2022 •

edited

Loading

odashi left a comment •

edited

Loading

odashi commented Nov 16, 2022 •

edited

Loading

jianan1104 commented Nov 16, 2022 •

edited

Loading

jianan1104 commented Nov 16, 2022 •

edited

Loading