Mathics3 / mathics-core

An open-source Mathematica. This repository contains the Python modules for WL Built-in functions, variables, core primitives, e.g. Symbol, a parser to create Expressions, and an evaluator to execute them.
https://mathics.org
Other
786 stars 48 forks source link

Implement `Breakpoint[]` #1183

Closed aravindh-krishnamoorthy closed 3 days ago

aravindh-krishnamoorthy commented 4 days ago

This PR implements the Breakpoint[] functionality discussed in issue #1172.

With this implementation, it is possible to invoke a Python breakpoint from the mathics command line for debugging. Alternative debuggers can be loaded by setting the relevant Python environment variable PYTHONBREAKPOINT.

Please feel free to edit the description text.

rocky commented 4 days ago

@aravindh-krishnamoorthy I do have some small changes to suggest, like capitalization of the word Python and adding parenthesis around breakpoint().

I have just sent an invite as a collaborator to this project. If you would put in this PR as a branch of this repo, I think I can more easily edit it. Thanks, and sorry for having you do more work on this.

rocky commented 4 days ago

Also, about the CI breakage. I think this will be fixed soon. Sorry again for this.

rocky commented 4 days ago

Looks like this needs a merge or rebase from master. Unfortunately we have an API breaking change that was just made.

aravindh-krishnamoorthy commented 4 days ago

@aravindh-krishnamoorthy I do have some small changes to suggest, like capitalization of the word Python and adding parenthesis around breakpoint().

I have just sent an invite as a collaborator to this project. If you would put in this PR as a branch of this repo, I think I can more easily edit it. Thanks, and sorry for having you do more work on this.

@rocky thank you for your comments. I don't mind at all! I've other experience, but when it comes to Mathics and Python I'm a beginner and would love to have feedback to improve my work.

I understood your point on Python capitalisation. However, could you please use the "review" feature to advise me what you mean by "parenthesis around breakpoint())" Then, I could include your changes directly, and GitHub also acknowledges your review contribution in the commit message.

rocky commented 4 days ago

I think what we'll have to do here to give a Breakpoint[] example is set up some Python code that does not actually stop. For pytest, the mock library can be used.

mmatera commented 4 days ago

The problem seems to be that the docpipeline does not handle this kind of exception properly. We would need to modify it to support a test like this.

aravindh-krishnamoorthy commented 4 days ago

Unfortunately, I'm not sure what URL to provide here. Comments and suggestions on the URL and the dummy breakpoint are welcome!

rocky commented 4 days ago

Unfortunately, I'm not sure what URL to provide here. Comments and suggestions on the URL and the dummy breakpoint are welcome!

Use:

    ## <url>:trace native symbol:</url>

For an example, see https://github.com/Mathics3/mathics-core/blob/master/mathics/builtin/trace.py#L107

mmatera commented 4 days ago

Unfortunately, I'm not sure what URL to provide here. Comments and suggestions on the URL and the dummy breakpoint are welcome!

Use:

    ## <url>:trace native symbol:</url>

For an example, see https://github.com/Mathics3/mathics-core/blob/master/mathics/builtin/trace.py#L107

Maybe it could work a link to the debuger documentation. For example, https://python2-trepan.readthedocs.io/en/latest/commands/breakpoints/break.html

rocky commented 4 days ago

Unfortunately, I'm not sure what URL to provide here. Comments and suggestions on the URL and the dummy breakpoint are welcome!

Use:

    ## <url>:trace native symbol:</url>

For an example, see https://github.com/Mathics3/mathics-core/blob/master/mathics/builtin/trace.py#L107

Maybe it could work a link to the debuger documentation. For example, https://python2-trepan.readthedocs.io/en/latest/commands/breakpoints/break.html

If you want to link to a description of how Python's breakpoint() works, use something like https://docs.python.org/3/library/functions.html#breakpoint

While I am flattered to have a link to stuff I wrote, that link is to the Python2 (not current Python3) code and that specific text isn't going to be of much help here since this doesn't get you into the Python2 debugger.

Right now I wouldn't fret over the specific details of a link or no link. Debugging is going to change and (I hope) get greatly more useful. Changing or improving the link is not a big deal (in contrast to most other changes), and I imagine this is the kind of thing that one gets a feeling over time as to what would be the most helpful.

Better debugging is the one area I can see in the short term where we can do much much better than WMA. In fact, I would argue that just having Breakpoint[] in there, as is, is already an improvement!

rocky commented 4 days ago

@mmatera The intermittent TimeConstraint failures in this and in another PRs are getting annoying. Any suggestions on how to address? Mine is to mark it skipped until it is no longer flaky.

mmatera commented 3 days ago

@mmatera The intermittent TimeConstraint failures in this and in another PRs are getting annoying. Any suggestions on how to address? Mine is to mark it skipped until it is no longer flaky.

Just by making the time in the second argument shorter. It seems that something became faster since I added this tests...

aravindh-krishnamoorthy commented 3 days ago

Perfect! I hope you find the feature useful. I just edited the previous comment to say that multi-line suggestions are also possible: Multi-line suggestions are also possible: https://stackoverflow.com/questions/60311158/how-can-i-suggest-multiple-lines-be-changed-in-markdown

Also, if you would prefer so, I could also commit the changes based on the review. However, as you suggested, I will start working directly on the Mathics3/mathics-core repository from the next PR.

mmatera commented 3 days ago

Perfect! I hope you find the feature useful. I just edited the previous comment to say that multi-line suggestions are also possible: Multi-line suggestions are also possible: https://stackoverflow.com/questions/60311158/how-can-i-suggest-multiple-lines-be-changed-in-markdown

Also, if you would prefer so, I could also commit the changes based on the review. However, as you suggested, I will start working directly on the Mathics3/mathics-core repository from the next PR.

Ah, sorry, I didn't understand that. Indeed, it looks useful!

rocky commented 3 days ago

For me, this is okay to merge. There may be other changes down the line, but these are more tweak-kind of things.