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.

Sign up for GitHub

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

Jump to bottom

pyparsing 3.1.0 causing test failures in Markdown image inlining #293

Closed
tsibley opened this issue Jun 20, 2023 · 4 comments · Fixed by #294
Closed

pyparsing 3.1.0 causing test failures in Markdown image inlining #293

tsibley opened this issue Jun 20, 2023 · 4 comments · Fixed by #294
Assignees
@tsibley
Labels
bug Something isn't working

Comments

@tsibley
Copy link
Member

tsibley commented Jun 20, 2023

Failure started between our scheduled CI runs on Sunday (passing) and Monday (failing). The Python distributions installed (for the 3.7, Linux job) differ only in pyparsing version:

diff --git a/tmp/a b/tmp/b
index 5d14482..48d52a8 100644
--- a/tmp/a
+++ b/tmp/b
@@ -49,7 +49,7 @@ pycodestyle-2.9.1
 pycparser-2.21
 pyflakes-2.5.0
 pyjwt-2.7.0
-pyparsing-3.0.9
+pyparsing-3.1.0
 pytest-7.3.2
 pytest-forked-1.6.0
 python-dateutil-2.8.2

3.1.0 was released on Sunday, about 5 hours after our successful CI run.

Noticed this in #289 (comment).
@tsibley tsibley added the bug Something isn't working label Jun 20, 2023
@tsibley tsibley self-assigned this Jun 20, 2023
@tsibley
Copy link
Member Author

tsibley commented Jun 20, 2023

The changelog has a few things it might be, but nothing that jumps out as for sure the issue. Going to bisect down to the specific beta release between 3.0.9 and 3.1.0.

@tsibley
Copy link
Member Author

tsibley commented Jun 20, 2023

Something in 3.1.0b1, as 3.1.0a1 is ok.

@tsibley
Copy link
Member Author

tsibley commented Jun 20, 2023

Ah, the issue is that

![image node with title](image.png "title")

is now getting incorrectly parsed as:

ImageNode(alt='image node with title', url='image.png "title"', title=None)

i.e. with the title as part of the url attribute. That's not right!

@tsibley
Copy link
Member Author

tsibley commented Jun 20, 2023

Bisected to pyparsing/pyparsing@9d789cb.

tsibley added a commit that referenced this issue Jun 20, 2023
@tsibley
markdown: Fix parsing of ![alt](url "title") under pyparsing ≥3.1.0b1
7a08967 
The recent release of pyparsing fixed a bug in SkipTo¹ "that has been
there for a very very long time"² and which we were unintentionally
relying upon.  The fix caused our parsing of

    ![alt](url "title")

to break as SkipTo(White()) no longer matched due to our grammar
ignoring whitespace by default.  We didn't notice until 3.1.0 was
released a couple days ago.

Explicitly don't ignore whitespace for this grammar element.  This
change is backwards compatible with older versions of pyparsing.

Resolves <#293>.

¹ <pyparsing/pyparsing@9d789cbc>
² <pyparsing/pyparsing#475>
@tsibley tsibley mentioned this issue Jun 20, 2023
Merged
1 task
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@tsibley tsibley

Labels
bug Something isn't working
Projects
No open projects
Nextstrain planning (archived)
Archived in project
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

pyparsing 3.1.0 fix nextstrain/cli
1 participant
@tsibley