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

Recent version of pyparsing (3.0.2) doesn't work well with pydot #277

Closed
MarcCote opened this issue Oct 27, 2021 · 10 comments
Closed

Recent version of pyparsing (3.0.2) doesn't work well with pydot #277

MarcCote opened this issue Oct 27, 2021 · 10 comments

Comments

@MarcCote
Copy link

MarcCote commented Oct 27, 2021

Ref: pyparsing/pyparsing#319

Minimal example:

import pydot
pydot.graph_from_dot_data("graph G { b1 [label=<Hello World>]; }")
graph G { b1 [label=<Hello World>]; }
      ^                                                                        
Expected '{', found 'G'  (at char 6), (line:1, col:7)
@MarcCote
Copy link
Author

This issue is now resolved in pyparsing 3.0.3. That should probably be reflected in the requirements of pydot.

@rossbar
Copy link

rossbar commented Oct 27, 2021

I suspect this is the source of recent problems in the networkx test suite: networkx/networkx#5155

@rossbar
Copy link

rossbar commented Nov 18, 2021

Just to update - NetworkX is experiencing problems related to pydot and pyparsing version 3.0.6.

@ptmcg
Copy link

ptmcg commented Nov 19, 2021

I've looked at the networkx issue, and the issue is due to the tightening up of whitespace-skipping that was pushed out in 3.0.2. This is intentional behavior. To correct in pydot, add the two lines shown below to dot_parser.py (I can submit as a PR if you prefer):

noncomma = "".join([c for c in printables if c != ","])
alphastring_ = OneOrMore(CharsNotIn(noncomma + " "))
# ========== add the following two lines ==============
# override pyparsing tightened whitespace-skipping logic
alphastring_.skipWhitespace = True

CharsNotIn has always suppressed whitespace, but in pyparsing 3.0, it is intended to preserve this whitespace-skipping suppression when enclosed in a surrounding expression. My understanding of alphastring_ is that it should collect non-comma characters after skipping over any initial whitespace. Adding this line passes the test suite. (The added comment is optional, but I thought it would be useful to document this atypical attribute setting.)

This fix also will address issues with pydot in networkx (pyparsing/pyparsing#338).

@peternowee
Copy link
Member

peternowee commented Nov 19, 2021

@ptmcg Hi Paul, sorry I do not have time to really delve into this. About your suggested change, is it backwards-compatible with pyparsing < 3.0 ?

But even if you say it is, I feel that this is too risky of a change for a mere point release of pydot (1.4.3 for example). The test suite is not very complete. For example, I know there are still many cases in the open issue list that are not yet tested for. I also remember investigating a problem related to whitespace parsing earlier.

If this is too risky for a point release, then there could be an early release of pydot 2.x. This will mess up the schedule a bit, because only part of all the planned improvements have been merged so far, and the rest would need to be postponed to pydot 3.x then.

Or, there could be a point release pydot 1.4.3 just setting the requirement for pyparsing to <3.0 in various places to prevent that users use pydot <2.0 with pyparsing >=3.0. The definitive fix will only go in pydot 2.0-dev then, but at the speed with which things are going now, it might take a long time before that actually gets released.

Unfortunately, my situation is such that I really do not have time for any of this. I am almost getting kicked out on the streets, so my only priority at the moment is finding some stable source of income. Any serious help would be appreciated, though I doubt I can then immediately shift my focus back to pydot. Perhaps one of the previous maintainers needs to step in (@prmtl @johnyf @erocarrera) to either help directly or hand out the necessary rights to someone else. Sorry, I wish I could do more.

@ptmcg
Copy link

ptmcg commented Nov 19, 2021

If you prefer, pin the current pydot to pyparsing 2.4.7 in a minor release, and then when the pydot maintainers have time/resources for your next pydot major release, open up to pyparsing 3.0.6.

The change I've proposed to pydot will be backward compatible to 2.4.7. (In that release, it is essentially a no-op.) I'll submit it in a PR so that it can be maintained for when someone gets a chance to revisit it.

I also have some other suggestions for the pydot parser (such as using Keyword instead of Literal for dot language keywords, and tighter expressions for real numbers and identifiers). I'll package them up in a separate PR for future consideration.

I'm sorry to hear about your personal circumstances. I strongly encourage you to delegate this off to a co-maintainer so you can focus on the important stuff. This is all just 1's and 0's, and comes a distant second to taking care of yourself.

gentoo-bot pushed a commit to gentoo/gentoo that referenced this issue Dec 3, 2021
Bug: pydot/pydot#277
Signed-off-by: Sam James <sam@gentoo.org>
@blshkv
Copy link

blshkv commented Jan 4, 2022

so what's the story here? Can you fix the issue and release a new version, compatible with pyparseing-3?

@lkk7
Copy link
Member

lkk7 commented Dec 16, 2023

Fixed. As Peter said, that may be risky. But better than the project being forgotten

@lkk7 lkk7 closed this as completed Dec 16, 2023
@blshkv
Copy link

blshkv commented Dec 21, 2023

a new release would be nice too.

@lkk7
Copy link
Member

lkk7 commented Dec 21, 2023

@blshkv yes, working on that. Should come out very soon (but after the upcoming holidays for sure). It hasn't been done yet because I don't have the permissions to do that.

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

No branches or pull requests

6 participants