Exception on parsing dot file #171
Comments
|
Thanks for reporting these errors. The digraph qnet {
rankdir="TB";
graph [pad="0", ranksep="0.25", nodesep="0.25"];
node [penwidth=0.5, height=0.25, color=black, shape=box, fontsize=10, fontname="Vera Sans, DejaVu Sans, Liberation Sans, Arial, Helvetica, sans"];
edge [penwidth=0.5, arrowsize=0.5];
compound=true;
subgraph cluster_qnet {
fontname="Vera Sans, DejaVu Sans, Liberation Sans, Arial, Helvetica, sans";
fontsize=12;
label="qnet";
tooltip="qnet";
subgraph cluster_algebra {
toolbox [target="_top", tooltip="qnet.algebra.toolbox", href="../API/qnet.algebra.toolbox.html"];
library [target="_top", tooltip="qnet.algebra.library", href="../API/qnet.algebra.library.html"];
core [target="_top", tooltip="qnet.algebra.core", href="../API/qnet.algebra.core.html", width=1.3];
pattern_matching [target="_top", tooltip="qnet.algebra.pattern_matching", href="../API/qnet.algebra.pattern_matching.html", width=1.3];
toolbox -> library;
toolbox -> core;
toolbox -> pattern_matching;
library -> core;
core -> pattern_matching [weight=0, minlen=0];
library -> pattern_matching;
href = "../API/qnet.algebra.html"; target="_top";
label="algebra";
tooltip="qnet.algebra";
graph[style=filled, fillcolor="#EEEEEE"];
}
convert [target="_top", tooltip="qnet.convert", href="../API/qnet.convert.html"];
visualization [target="_top", tooltip="qnet.visualization", href="../API/qnet.visualization.html"];
printing [target="_top", tooltip="qnet.printing", href="../API/qnet.printing.html"];
utils [target="_top", tooltip="qnet.utils", href="../API/qnet.utils.html", width=1];
{ rank=same; convert[width=0.8]; visualization[width=0.8]; printing[width=0.8]; }
convert -> visualization [minlen=3, style=invis];
visualization -> printing [minlen=3];
visualization -> toolbox [minlen=2, lhead=cluster_algebra];
printing -> toolbox [lhead=cluster_algebra];
convert -> toolbox [lhead=cluster_algebra];
core -> utils [ltail=cluster_algebra];
pattern_matching -> utils [ltail=cluster_algebra];
convert -> utils [minlen=6];
printing -> utils [minlen=6];
}
}A minimal example that demonstrates this issue is: graph {
foo[bar; bar]
}which raises graph {
foo[bar, bar]
}which is parsed without errors. The cause of the error appears to be the parsing rule that defines what an attribute list is: In particular, the expression
|
|
There is still a bug lurking here, in the code handling of the origina-exception, Lines 551 to 554 in 48ba231 It should become: print("%s%s^%s" % (err.line, " "*(err.column-1), err)) |
|
But it is better to let the exception bubble-up, or else, the user will receive a |
to let exception bubble-up.
|
@ankostis In my PR #219 I also worked on this part of the code. I will reply to your suggestions here: First, replying to #171 (comment):
This is an alternative solution for the I am still missing the newlines though. Aren't you? Perhaps some Linux/Windows difference? See https://github.com/pydot/pydot/pull/219/files#diff-baef597193866f900a2726a8a4667b12R556 for my solution which also adds newlines. Second, replying to #171 (comment):
and to #171 (reference) which points to your monkey patch in the pydot-using software pygraphkit/graphtik@3efd534 which patches out the pydot Letting the exception bubble-up sounds reasonable, but perhaps this can also be accomplished by adding a Your patch also shows that you prefer pydot not to print details of the Hope you can let me know what you think. Thanks. |
|
Thank you for you thoughtful reponse.
The "end-users" of this library are supposed to be other developers, not final users, correct? Now there is a highly problematic point in Python srtandard library, when calling out other processes with Now, having written Py2/Py3 & Windows/POSIX compatible-code to collect & attach STDOUT, STDERR & cmd-line in a custom exception, i can tell, it is not an easy task
That way, this library would still raise self-evident exceptions, and avoid printing spurious error-messages to (btw, shouldn't in print |
|
My commenta above expressed in this review: #219 (review) |
Hmm, not really, sorry. At least, I did not mean the developers that use pydot directly. For the problem I described, I was thinking of indirect pydot users, which could be final users, but also include developers in case there are multiple layers between pydot and the final end-user. I mean all the ones that are more than one layer below pydot. So, first, one layer below pydot, you have the developer that directly uses pydot to quickly write a library or program. We can expect him to learn how to troubleshoot pydot, but should we also expect him to add handling for all the possible exceptions that pydot may raise before he publishes his software? Do these automatically bubble-up if he does nothing? Won't he ruin the pydot exception by raising his own exception instead of chaining? Will he keep his handling code up-to-date with changes to pydot exception classes (I know, we should ensure backwards compatibility, but still)? Then, on all the layers below that, so two or more layers below pydot, are the indirect users of pydot. If pydot does not print, and somewhere in the chain of the libraries and programs between pydot and the final user, the exception that pydot raised is dropped, then these indirect users are in a bad position, developer or not. They do not know pydot yet and are suddenly confronted with an exception without any details. OTOH, we could just say that this is the fault of those developers in-between and that we only support the direct users of pydot. When indirect users or end-users file support questions at pydot, we can always tell them to go ask the intermediary developers first. However, that may not always be easy for the end-user. And the triaging of those initial support requests at pydot also costs time. I do not know how often this all occurs, so maybe it is a bit of a theoretical discussion, a bit too pessimistic, but I just thought we could try a middle way first, like that debug/verbose-flag with a default of True mentioned in my comment above, so that direct pydot-using developers can flip it to False themselves once they are confident about their own handling of pydot exceptions. I think we can agree on how things should be done, but we cannot control the developers between pydot and the final user and therefore I am a bit weary of what it would lead to in practice. You seem to have more practical experience, so please tell me if you are more optimistic.
In principle, yes, I have been thinking a lot about that as well, see #218 (comment) for my earlier attempt to start this discussion. I was awaiting a response from this project's maintainer @prmtl before writing more about the details. I think you and I are both thinking in the same direction.
Your code there seems to be in the direction that I was thinking about as well, yes: A custom exception class containing everything we want the end user to know. Here are some of the sources I read about this last year:
By the way, there is still an unused class lying around in the pydot code: Back to your comment about compatibility, I can imagine that this is an issue. Last year, I looked at I did not know about the Windows/POSIX differences, but can also imagine they exist, especially when it comes to OS-related exceptions. Maybe in this case of
Yes, I see the benefit.
I found it was difficult to find sources on the general notion of library output, but from what I did find, I understand that printing to With regard to the
Ok, I will reply there as I go. I can see already that some of your suggestions require some more investigation. I do not always have as much time. Thanks for the interesting discussion. :) |
|
[edit:] Thank you for the references to SO for the general problems. IntroAs they say, there are just 2 hard problems in programming:
I'm particularly intrigued by the 2nd class of errors, because it is intimately linked to the architecture of program. Actually 80% of code we write is to convey to fellow programmers our thoughts, and to control their reactions to unseen cases (i.e errors). Error handling & LoggingI'm picking some interesting points you raised to respond:
👍
Well, it's not that hard, because for library code you should not fiddle with logging configurations - that's the responsibility of downstream programmers, for the app facing the final-user. logger = logging.getLogger(__name__)
...
log.info("Hello %s", "world")
...
log.info("Terrible thing happened: %s", "a very long shit!", exc_info=1)
...
log.debug("Study this stacktrace:", stack_info=1) (*) to be pedantic, this pattern is not completely correct, but 99% of code out there does that, and it's bad only for long running processes.
Well, it's not that bad, and certainly it's much better than polluting
Yes, they are lost. No, they do not end upo to syslog, unless an intermediate developer installs the respective log-handler for that. [edit] Or the library ends up running in some daemon with stderr redirected to syslog.
The good thing about
Absolutely 👍👍 I suggest then to take a look at my Bonus track: Are chained-exceptions over-rated?When you develop a library you still need to configure logging subsystem, logging.basicConfig(level=DEBUG)But as i stressed, lib-devs must stay away from configuring the logging system. And should refrain from interpreting the errors the lb generates - these are not their business. ok= False
try:
// do somtehing
ok = True
finally:
if not ok:
//clean up |
|
Funny that the apparent error in your quote about hard problems in programming is actually hard to handle for me. How to respond to this? Let me just say your memory of the quote may no longer be valid and should be refreshed next time you read it from disk. :) For the rest, I just wanted to say that I have read your reply and it all looks very interesting. I will study and think about these subjects a bit more when I have the time. My schedule is quite unpredictable, so no need to wait for me if you get a chance to make improvements you see fit. |
|
Let's hope you still have some time left for treading the original quotes :-) |
|
Haha, cool, maybe one day your version will make it to that page as well. |
|
I guess that from the start, you wanted me to raise it. |
When parsing DOT strings or files, pydot's parser calls the third-party library PyParsing. If PyParsing meets a problem, it raises a `ParseException`, which pydot then uses to report some details. Currently, during the handling of that exception, a `TypeError` occurs. It is caused by an attempt to concatenate a string with the exception using the `+` (addition/plus) operator, which is (not allowed)[1]. The problem was introduced in 2016 by commit (b4a3810)[2]. This current commit now removes that concatenation and goes back to printing three separate lines. The end result is the same as Lance Hepler's (original 2013 solution)[3] and (the example code in the PyParsing documentation)[4]. I tested that behavior is now as expected (printing parser error details without `TypeError`) under Python 2.7.16 with PyParsing 2.4.2 and Python 3.7.3 with PyParsing 2.4.2 and 3.0.0a2 ((486b1fd2e)[5]). Unit tests all pass (same three combinations, when run together with the test suite fix proposed in PR 211). This (closes pydot#176)[6] ("Error in error handling hides the real error message"), reported by Shish, and resolves the `TypeError` part of (issue pydot#171)[7] ("Exception on parsing dot file"), reported by Michael Goerz. [1]: https://docs.python.org/3/reference/expressions.html#binary-arithmetic-operations [2]: pydot@b4a3810#diff-baef597193866f900a2726a8a4667b12R563-R566 [3]: nlhepler/pydot@687eb7e#diff-baef597193866f900a2726a8a4667b12R551-R553 [4]: pyparsing/pyparsing@2c6f881#diff-7ab8c6f0d66d170e10b60b9f65334e18R694-R697 [5]: https://github.com/pyparsing/pyparsing/tree/486b1fd2ef7e98966665f915bc59856996ffb5b0 [6]: pydot#176 [7]: pydot#171
When parsing DOT strings or files, pydot's parser calls the third-party library PyParsing. If PyParsing meets a problem, it raises a `ParseException`, which pydot then uses to report some details. Currently, during the handling of that exception, a `TypeError` occurs. It is caused by an attempt to concatenate a string with the exception using the `+` (addition/plus) operator, which is [not allowed][1]. The problem was introduced in 2016 by commit [b4a3810][2]. This current commit now removes that concatenation and goes back to printing three separate lines. The end result is the same as Lance Hepler's [original 2013 solution][3] and [the example code in the PyParsing documentation][4]. I tested that behavior is now as expected (printing parser error details without `TypeError`) under Python 2.7.16 with PyParsing 2.4.2 and Python 3.7.3 with PyParsing 2.4.2 and 3.0.0a2 ([486b1fd2e][5]). Unit tests all pass (same three combinations, when run together with the test suite fix proposed in PR 211). This [closes pydot#176][6] ("Error in error handling hides the real error message"), reported by Shish, and resolves the `TypeError` part of [issue pydot#171][7] ("Exception on parsing dot file"), reported by Michael Goerz. [1]: https://docs.python.org/3/reference/expressions.html#binary-arithmetic-operations [2]: pydot@b4a3810#diff-baef597193866f900a2726a8a4667b12R563-R566 [3]: nlhepler/pydot@687eb7e#diff-baef597193866f900a2726a8a4667b12R551-R553 [4]: pyparsing/pyparsing@2c6f881#diff-7ab8c6f0d66d170e10b60b9f65334e18R694-R697 [5]: https://github.com/pyparsing/pyparsing/tree/486b1fd2ef7e98966665f915bc59856996ffb5b0 [6]: pydot#176 [7]: pydot#171
|
@ankostis: I have read up on logging and exception handling a bit and I have seen 'some' light. Thank you for your pointers. I now agree with most of what you said:
However, I am still not sure about what exception to raise exactly. I still want to help that unlucky end-user that gets confronted with it. For example, you wrote:
and
Is there room for an exception to those rules here? Because I do have some idea on what to do with the error and I do think we can actually add some valuable interpretation, like:
I have drafted a basic troubleshooting guide to be included in the pydot documentation that:
I drafted it now as a new paragraph in the The only thing I still need now, is a way to point the user to that documentation. If we just allow the original exception to bubble up without any additional information, users with little experience reading tracebacks or otherwise unaware of pydot's role, will not immediately look at our troubleshooting guide. They will just start searching for the term Some of the ways I considered to point the user to our documentation:
To conclude the above list: I am currently thinking of a custom exception class with our own message and carrying only our additional attributes, chained to the original exception which has its own attributes. And for now no integration between the custom exception classes needed for PR 218 and PR 219. About the contents of our message, I think it can contain a short interpretation from our side (e.g. Example using Now the error ends in a clear message pointing to the pydot documentation. Alternatively, using a custom exception with a unique name that should help users find pydot documentation by themselves: This last one is my current preference. It inherits from I hope you can let me know what you think. Replying to your other comments:
Ok, I will assume Python 3.5+ in #218 and #219 from now on (next force-push). For Python 2, I created a very targeted bug fix for inclusion in a bug fix/point release in #227.
This is relevant only to PR #218, I think. I had a quick look and I will look at it in more detail later and see what I can re-use, but I can already tell you that I don't really feel comfortable monkey-patching the standard library, for the usual risks involved. I would rather use a custom exception, I think. Also, I have come to accept that not all of our additional data needs to be part of the exception string, so maybe there is no need to override
Ok, I still need to look at testing. I will come back on this if I have any questions later.
Yes, that should do the trick in many cases. I have put this in the proposed documentation as the first method to try when someone wants to see the DEBUG lines. But there are two possible downsides:
Therefore, I plan to document the following alternative as well: logging.getLogger('pydot').addHandler(logging.StreamHandler())
logging.getLogger('pydot').setLevel(logging.DEBUG)
logging.getLogger('pydot.pydot').setLevel(logging.DEBUG)
logging.getLogger('pydot.dot_parser').setLevel(logging.DEBUG)Notes:
Finally, two other issues that were brought up earlier in the context of PR 218 and/or PR 219 and on which my thoughts have started shifting:
|
|
What an excellent analysis document! 😄 Here are my thoughts: Comments in the raising code
Actually that's a neat way to add human context to the error without cluttering the exception workload. If I would order the channels to convery messages to the user, it is the following:
Chained-exceptionYes, when original exception curries a big load, like pyparsing, but it might be irrelevant to you code, chain it. My preferred way is to store all info in the original I would inherit ValueError for the custom error (since it is a problem in the input). Another point: My convention is always to include the crux of the chained exception into the top one, except Exception as ex:
... # contrcuct context infos
raise TopException(f"Graphviz failed due to: {ex}", cwd, stder, ...) from exPackage & module organizationI agree, the current organization of python files is "unusual", and complicates crafting a sensible exception hierarchy. If The Exceptions should be definitely defined in the package-module. It is tempting to copy-paste the whole
Copy all in the Python 3.X compatibilityPlease, lease, consider PY3.6+!
Anything before PY3.6 is legacy, along with PY2.7. logging
But that is exactly the main architectural design of logging subsystems, For the package structure as it is, i would recommend just the in the docs just: logging.basicConfig(level=0)
logging.getLogger().level=0) # in case already configuredand leave all the rest (i.e. reduce clutter) for the curious user to figure it herself :-) |
Thank you and thank you for sharing your thoughts. My reply again:
Great minds think alike. :) But innovative as it may be, I found the resulting stack trace still too obscure for the user and the use of comments for this purpose too unconventional for a project with multiple contributors. I concluded that my goal of helping out the inexperienced user was better accomplished by having the message in the final line.
With those first two items you also mean final exceptions that have earlier exceptions chained to them, right? Not just stand-alone exceptions, right?
[...]
Why choose
Yes, I certainly plan to do that for the parsing problem (#219). For the Graphviz problems (#218), I still need to check if the problem is always in the input. Perhaps I can differentiate between different possible problems (e.g. DOT-syntax problem, temporary .dot-file not found, Graphviz executable not found, maybe more). It will take some time to investigate this. After that, we can discuss further in #218 if necessary.
Yes, ok, this is possible, but I guess it depends on the case a little bit. It may be useful in cases where we are forced to use a custom exception because we want to add some data, but we do not have anything interesting to add to the original message, perhaps in the case of #218 (Graphviz/ However, in cases where we can provide the user with a better explanation, such as this #171 and #219 (pyparsing/ So, thanks for the tip, I will use it where appropriate.
I am sorry, but as I said in my previous comment, due to time constraints I hope to update my PRs #218 and #219 separate of the bigger questions related to package and module organization. Making the PRs depend on big questions like this would unnecessarily hamper progress, in my eyes. That is why I proposed to use separate, independent custom exceptions in Of course, I will try to anticipate on the outcomes of the bigger questions, which is why I proposed to use identical names for identical attributes and to add comments near the exception class definitions to make other developers aware of the possible future integration. If you have anything to add to that, you can still let me know. Back to the package & module organization: How about we create a new issue for that? Would you like to do that (or maybe even a PR) or shall I do it? You seem to have more experience with this subject than I do. If I would have to create a new issue for it now, it would be nothing more than a reminder/todo-item with just a copy of your above comments. It would probably take me weeks, if not months, to push out a first proposal, while you might be able to come up with a proposal in just a few days.
Difficult question. Some additional arguments: In favor of dropping PY3.5 support:
Against dropping PY3.5 support:
Inconclusive:
So, I am still hesitant. Do you use PY3.6-specific functionality in your open PR now, or need it for something else you are working on? If not, I propose we assume PY3.5 support for now and reconsider again when the need arises, or in September 2020. Is that ok?
Yes, that works as long as the child loggers are still NOTSET, but you left out the rest of the sentence:
Remember, this is for use in the documentation on how to get access to the DEBUG log lines as a second, alternative method to try if the first method did not work. Therefore, it has to be stronger and assume things like child loggers not inheriting from the parent because their level is not NOTSET anymore.
I am open for suggestions and willing to consider changes to the logger hierarchy and logger names as long as my changes have not been merged. What logger hierarchy and names did you expect instead? Something like
Yes, handcrafted.
I don't know about "always". If we agree that the current package structure is "unusual" and are thinking about changing that structure (see Package & module organization above), we could already try to let the logger-names anticipate the most likely future package structure by now hard-coding what we expect I understand that there is a risk that we guess wrong, but that risk needs to be weighted against the costs of waiting for the new structure. If someone thinks the new structure should be implemented first, they should contribute to that (again, see Package & module organization above). I am willing to rebase and reconsider the logging setup in my PRs as long as they have not been merged.
Yes, ok, this will work in case:
Hmm, I don't know. We are talking about a section in the documentation that is dedicated to getting the DEBUG log lines out. I think there is room for giving an second, alternative method in case the first method did not work. I will try not let it result in clutter. |
|
I agree almost wholeheartedly with everything you concluded. It's just your hesitation for "what if the logger has stopped being NOTSET" seems rather peculiar. Don't forget, libraries don't touch logging configs (to avoid imbroglios like these). I did that in the past, and tried to anticipate the problems novice users would face in my final application, Nowdays, i rarely point to some considerable important 3rdp document - i don't write instructions for other libraries because then i have to maintain instructions about other people's business :-) And you are not even talking about a final application here. If you ask me, even the 2-line example code i gave is too much, not fitting for a library project Given the opportunity, very welcomed to foresee the proper package structure in the logger-names. |
Good, thank you, happy to hear that. Thank you for providing new insights. I learned a lot the last few weeks.
Yes, I was thinking of the final application mostly. I only started studying logging a few weeks ago, so I have not seen much of how final applications use it in practice, but the fear that they might change library logging levels originated from the following things I saw and read:
Anyway, I hope that explains my fear that our loggers may get disabled in ways that the first method (
Hehe, funny. Exceptional question and great response.
Yes, ok, I see. Thanks for sharing your experiences and the balance you found. Very valuable. I will need to find a similar balance. Specifically with regard to this case, I guess I should also consider that anything I put in the documentation will need to be maintained later as well (e.g. if child logger names get changed in the code). Maybe I will just start out with only the
Ok, I will try, probably something like About that discussion on package structure (Package & module organization): I still think that deserves a separate issue or PR. Do you want to work on that, or shall I just open an issue for anyone to pick up? |
Reasons why support for Python 3.5 is not dropped yet: - We do not urgently need Python 3.6+ specific functionality. - Python 3.5 is still receiving security support until 2020-09-13 [1]. - Recent PyPI download statistics for pydot [2] show that 20 to 40% of the users are still on Python 3.5. For further details, see [pydot#171][3]. [1]: https://devguide.python.org/#status-of-python-branches [2]: https://pypistats.org/packages/pydot [3]: pydot#171 (comment)
Why support for Python 3.5 is not dropped yet: - We do not urgently need Python 3.6+ specific functionality. - Python 3.5 is still receiving security fixes until 2020-09-13 [1][2]. - Recent PyPI download statistics for pydot [3] show that 20 to 40% of the users are still on Python 3.5. For further details, see [pydot#171][4]. [1]: https://en.wikipedia.org/wiki/History_of_Python#Table_of_versions [2]: https://devguide.python.org/#status-of-python-branches [3]: https://pypistats.org/packages/pydot [4]: pydot#171 (comment)
pydot currently prints some error messages to standard output (`stdout`). Printing anything is considered bad practice for libraries [1] [2]. This becomes even more disturbing if the prints contain long Graphviz output or DOT strings. [1]: https://stackoverflow.com/questions/4201856/error-handling-strategies-in-a-shared-library-c [2]: https://www.reddit.com/r/C_Programming/comments/97ebfj/redirect_stdout_and_stderr_to_a_buffer/e47n9kh/ This commit lays the groundwork for switching from printing to logging by providing a basic library logging setup and some user documentation on how to read the logs. Later commits will replace the actual printing by logging. Discussion: pydot#171.
|
Regarding the pesky I have not ever seen any code doing that :-). |
|
Yes, thank you, I had come across that as well. It is quite smart, but also very unpractical, requiring |
pydot currently prints some error messages to standard output (`stdout`). Printing anything is considered bad practice for libraries [1] [2]. This becomes even more disturbing if the prints contain long Graphviz output or DOT strings. [1]: https://stackoverflow.com/questions/4201856/error-handling-strategies-in-a-shared-library-c [2]: https://www.reddit.com/r/C_Programming/comments/97ebfj/redirect_stdout_and_stderr_to_a_buffer/e47n9kh/ This commit lays the groundwork for switching from printing to logging by providing a basic library logging setup and some user documentation on how to read the logs. Later commits will replace the actual printing by logging. Discussion: pydot#171.
When parsing DOT strings or files, pydot's parser calls the third-party library PyParsing. If PyParsing meets a problem, it raises a `ParseException`, which pydot then uses to report some details. Currently, during the handling of that exception, a `TypeError` occurs. It is caused by an attempt to concatenate a string with the exception using the `+` (addition/plus) operator, which is [not allowed][1]. The problem was introduced in 2016 by commit [b4a3810][2]. This current commit now removes that concatenation and goes back to printing three separate lines. The end result is the same as Lance Hepler's [original 2013 solution][3] and [the example code in the PyParsing documentation][4]. I tested that behavior is now as expected (printing parser error details without `TypeError`) under Python 2.7.16 with PyParsing 2.4.2 and Python 3.7.3 with PyParsing 2.4.2 and 3.0.0a2 ([486b1fd2e][5]). Unit tests all pass (same three combinations, when run together with the test suite fix proposed in PR 211). This [closes pydot#176][6] ("Error in error handling hides the real error message"), reported by Shish, and resolves the `TypeError` part of [issue pydot#171][7] ("Exception on parsing dot file"), reported by Michael Goerz. [1]: https://docs.python.org/3/reference/expressions.html#binary-arithmetic-operations [2]: pydot@b4a3810#diff-baef597193866f900a2726a8a4667b12R563-R566 [3]: nlhepler/pydot@687eb7e#diff-baef597193866f900a2726a8a4667b12R551-R553 [4]: pyparsing/pyparsing@2c6f881#diff-7ab8c6f0d66d170e10b60b9f65334e18R694-R697 [5]: https://github.com/pyparsing/pyparsing/tree/486b1fd2ef7e98966665f915bc59856996ffb5b0 [6]: pydot#176 [7]: pydot#171
This starts a series of commits to drop support for Python 2 and 3.4 as discussed in [pydot#171][171] and [pydot#229][229]. | Python version | Python EOL | PyPI 2020-05-02 | PyPI 2021-06-21 | |----------------|------------|-----------------|-----------------| | Python 2.7 | 2020-01-01 | 26% | 12% | | Python 3.4 | 2019-03-18 | 0% | 0% | | Python 3.5 | 2020-09-30 | 27% | 3% | | Python 3.6 | 2021-12 | 21% | 14% | | Python 3.7 | 2023-06 | 23% | 51% | | Python 3.8 | 2024-10 | 3% | 13% | | Python 3.9 | 2025-10 | 0% | 6% | | Python 3.10 | 2026-10 | - | 0% | EOL : End of life, from End of security support on [Wikipedia][1]. PyPI: Python Package Index statistics for pydot from [PyPIstats.org][2]. Without support for Python 2, wheel distributions of pydot can [no longer be marked as "universal"][3], so removing that from `setup.cfg`. **USER FEEDBACK REQUESTED** We are considering if pydot 2.0 should drop support for Python 3.5 and 3.6 as well. If this would affect you, please leave a comment in [pydot#268][268]. [1]: https://en.wikipedia.org/w/index.php?title=History_of_Python&oldid=1022680403#Table_of_versions [2]: https://pypistats.org/packages/pydot [3]: https://github.com/pypa/packaging.python.org/blob/44313e5db4d729eede0bcb91c08d6ec93e89c5c8/source/guides/dropping-older-python-versions.rst#dealing-with-the-universal-wheels [171]: pydot#171 (comment) [229]: pydot#229 [268]: pydot#268
This starts a series of commits to drop support for Python 2 and 3.4 as discussed in [pydot#171][171] and [pydot#229][229]. | Python version | Python EOL | PyPI 2020-05-02 | PyPI 2021-06-21 | |----------------|------------|-----------------|-----------------| | Python 2.7 | 2020-01-01 | 26% | 12% | | Python 3.4 | 2019-03-18 | 0% | 0% | | Python 3.5 | 2020-09-30 | 27% | 3% | | Python 3.6 | 2021-12 | 21% | 14% | | Python 3.7 | 2023-06 | 23% | 51% | | Python 3.8 | 2024-10 | 3% | 13% | | Python 3.9 | 2025-10 | 0% | 6% | | Python 3.10 | 2026-10 | - | 0% | EOL : End of life, from End of security support on [Wikipedia][1]. PyPI: Python Package Index statistics for pydot from [PyPIstats.org][2]. Without support for Python 2, wheel distributions of pydot can [no longer be marked as "universal"][3], so removing that from `setup.cfg`. **USER FEEDBACK REQUESTED** We are considering if pydot 2.0 should drop support for Python 3.5 and 3.6 as well. If this would affect you, please leave a comment in [pydot#268][268]. [1]: https://en.wikipedia.org/w/index.php?title=History_of_Python&oldid=1022680403#Table_of_versions [2]: https://pypistats.org/packages/pydot [3]: https://github.com/pypa/packaging.python.org/blob/44313e5db4d729eede0bcb91c08d6ec93e89c5c8/source/guides/dropping-older-python-versions.rst#dealing-with-the-universal-wheels [171]: pydot#171 (comment) [229]: pydot#229 [268]: pydot#268
|
Hi @SHuang-Broad, I think the issue you reported in #269 is a duplicate of this one. Look at this comment of @johnyf in 2018 regarding the Back to the Until then, hope you can use the workaround mentioned by @johnyf: Use a comma |
This commit brings the directory layout in line with current
recommendations by the Python Packaging Authority and others.
Before After
|-- setup.py |-- setup.py
| |-- src/
| | `-- pydot/
| | |-- __init__.py
|-- pydot.py | |-- core.py
|-- dot_parser.py | `-- dot_parser.py
`-- test/ `-- test/
`-- pydot_unittest.py `-- pydot_unittest.py
There are many opinions on what would be the best directory layout for
a Python project, particularly on whether or not to use an additional
`src/` layer. For example:
- https://blog.ionelmc.ro/2014/05/25/python-packaging/#the-structure
- pypa/packaging.python.org#320
- https://github.com/pypa/packaging.python.org/blob/7866cb69f1aa290a13c94da77cfb46a079cfcfa2/source/tutorials/packaging-projects.rst#a-simple-project
- https://stackoverflow.com/questions/193161/what-is-the-best-project-structure-for-a-python-application
Suffice to say, I can basically understand the arguments in favor of a
`src/` layout and do not find the arguments against it strong enough
not to use it.
Moved the dunder global variables (`__version__` etc.) to `__init__.py`
to ensure they remain available to the user doing `import pydot`. (They
would not be included in `from pydot.core import *` because their names
start with an underscore and I did not want to start keeping `__all__`
just for this.) A test for `pydot.__version__` is added to the
testsuite as well.
.... TODO: Something about separating parsing from core or not? ...
Discussed in pydot#171, pydot#230 and pydot#271.
Special thanks to Kostis Anagnostopoulos (@ankostis) for his advice.
- Module structure following example from NetworkX and other projects.
This adds an `import pydot` to `core.py`, which is cyclic/circular,
but necessary to make the exceptions available. Cyclic imports are
not uncommon and Python can handle this. Using the exceptions in
pydot source code now requires prefixing `pydot.`, for example:
raise pydot.Error("Message")
- Exception hierarchy changes. See ChangeLog and class docstrings in
this commit for details. Additional background notes:
- Removal of the unused exception class `InvocationException`: It was
introduced in 842173c of 2008 (v1.0.2), but got in disuse when
commits 9b3c1a1 and bc639e7 of 2016 (v1.2.0) fell back to using
`Exception` and `assert` (`AssertionError`) again. If we ever need
a custom class like this again in the future, it would probably
have a different signature for context data (e.g. a DOT string,
input and output), different parent class (or classes, e.g.
`PydotException, CalledProcessError`) and perhaps a different name
(e.g. ending in `...Error`), so no need to keep the old class
around for that.
Further additions to the exception hierarchy are likely before the
final release of pydot 2.0.
Discussed in pydot#171, pydot#230 and pydot#271.
This commit brings the directory layout in line with current
recommendations by the Python Packaging Authority and others.
Before After
|-- setup.py |-- setup.py
| |-- src/
| | `-- pydot/
| | |-- __init__.py
|-- pydot.py | |-- core.py
|-- dot_parser.py | `-- dot_parser.py
`-- test/ `-- test/
`-- pydot_unittest.py `-- pydot_unittest.py
There are many opinions on what would be the best directory layout for
a Python project, particularly on whether or not to use an additional
`src/` layer. For example:
- https://blog.ionelmc.ro/2014/05/25/python-packaging/#the-structure
- pypa/packaging.python.org#320
- https://github.com/pypa/packaging.python.org/blob/7866cb69f1aa290a13c94da77cfb46a079cfcfa2/source/tutorials/packaging-projects.rst#a-simple-project
- https://stackoverflow.com/questions/193161/what-is-the-best-project-structure-for-a-python-application
Suffice to say, I can basically understand the arguments in favor of a
`src/` layout and do not find the arguments against it strong enough
not to use it.
Moved the dunder global variables (`__version__` etc.) to `__init__.py`
to ensure they remain available to the user doing `import pydot`. (They
would not be included in `from pydot.core import *` because their names
start with an underscore and I did not want to start keeping `__all__`
just for this.) A test for `pydot.__version__` is added to the
testsuite as well.
A question that remains open for the moment is whether we should reduce
the weight of pydot "core" by splitting off functionality, most notably
the parsing functionality. Although that would reduce pydot loading
time, it would also make it more difficult for users to discover and
use that functionality. For details see:
pydot#271 (comment)
For now, things are kept as-is, namely that `pydot` always imports
`dot_parser`. This is now accomplished via `__init__.py` and `core.py`.
Discussed in pydot#171, pydot#230 and pydot#271.
Special thanks to Kostis Anagnostopoulos (@ankostis) for his advice.
- Module structure following example from NetworkX and other projects.
This adds an `import pydot` to `core.py`, which is cyclic/circular,
but necessary to make the exceptions available. Cyclic imports are
not uncommon and Python can handle this. Using the exceptions in
pydot source code now requires prefixing `pydot.`, for example:
raise pydot.Error("Message")
- Exception hierarchy changes. See ChangeLog and class docstrings in
this commit for details. Additional background notes:
- Removal of the unused exception class `InvocationException`: It was
introduced in 842173c of 2008 (v1.0.2), but got in disuse when
commits 9b3c1a1 and bc639e7 of 2016 (v1.2.0) fell back to using
`Exception` and `assert` (`AssertionError`) again. If we ever need
a custom class like this again in the future, it would probably
have a different signature for context data (e.g. a DOT string,
input and output), different parent class (or classes, e.g.
`PydotException, CalledProcessError`) and perhaps a different name
(e.g. ending in `...Error`), so no need to keep the old class
around for that.
Further additions to the exception hierarchy are likely before the
final release of pydot 2.0.
Discussed in pydot#171, pydot#230 and pydot#271.
pydot currently prints some error messages to standard output (`stdout`). Printing anything is considered bad practice for libraries [1] [2]. This becomes even more disturbing if the prints contain long Graphviz output or DOT strings. [1]: https://stackoverflow.com/questions/4201856/error-handling-strategies-in-a-shared-library-c [2]: https://www.reddit.com/r/C_Programming/comments/97ebfj/redirect_stdout_and_stderr_to_a_buffer/e47n9kh/ This commit lays the groundwork for switching from printing to logging by providing a basic library logging setup and some user documentation on how to read the logs. Later commits will replace the actual printing by logging. Discussed in pydot#171 and pydot#231.
* Implement basic library logging pydot currently prints some error messages to standard output (`stdout`). Printing anything is considered bad practice for libraries [1] [2]. This becomes even more disturbing if the prints contain long Graphviz output or DOT strings. [1]: https://stackoverflow.com/questions/4201856/error-handling-strategies-in-a-shared-library-c [2]: https://www.reddit.com/r/C_Programming/comments/97ebfj/redirect_stdout_and_stderr_to_a_buffer/e47n9kh/ This commit lays the groundwork for switching from printing to logging by providing a basic library logging setup and some user documentation on how to read the logs. Later commits will replace the actual printing by logging. Discussed in #171 and #231. * Fixup: Black reformatting * Fixup: Use `__name__` to name loggers * Fixup: Add parent logger, Reorder registration Notes: - I am aware that in `__init__.py`, the placement of the logger registration before the imports of the submodules leads to additional warnings in code checkers: From pylint: C0413: Import "from pydot.exceptions import *" should be placed at the top of the module (wrong-import-position) C0413: Import "from pydot.core import *" should be placed at the top of the module (wrong-import-position) From flake8: E402 module level import not at top of file E402 module level import not at top of file I do not really see how we can avoid this if we want to log a first message early during pydot initialization. Also note that the concerned imports are all pydot "internal" submodules and that for `__init__.py` importing those is its main task, so this having a central place in the file does not seem unnatural. - In `test/pydot_unittest.py`, added a final `importlib.reload(pydot)` to prevent subsequent tests from failing, probably caused by one of the caveats mentioned in: https://docs.python.org/3/library/importlib.html#importlib.reload * Fixup: Documentation reordering 1 * Fixup: Documentation reordering 2 * Fixup: Documentation changes (minor) Notes: - Removed the hint that the parent logger can be used to control all pydot loggers, because this is basic knowledge covered by the Python documentation to which we already refer. * Renumber links, Update PyPI URL in README.md * Fixup: update the changelog * Fixup: stabilize black usage --------- Co-authored-by: lkk7 <lukaszlapinski7@gmail.com>
With
structure.dotasI get the following exception:
That exception is actually raised during the handling of another Exception:
It doesn't seem to me that the file is an invalid dot file (and the graphviz command line utilities convert it fine)
The text was updated successfully, but these errors were encountered: