-
Notifications
You must be signed in to change notification settings - Fork 17
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
Use the doctest module in get_example_data #308
Conversation
They aren't actually that important for doctests because they are only used for the reporting, which we are bypassing anyways.
I think the main thing that needs to be done here is now is to take a look at the generated JSON and see if we like how it looks. It's not hard to change what is there. |
Doesn't do anything right now because success/failure isn't saved
To do for me:
|
You should also just review the code, and run this against the existing example configurations to make sure nothing funny is happening. |
I pushed a commit that inject a debug function instead of lambda s: None, It seem that some of the parsing is incorrect, as I get an $ papyri gen examples/papyri.toml --only papyri.examples:example1
...
Unexpected exception (<class 'SyntaxError'>, SyntaxError('multiple statements found while compiling a single statement', ('<doctest example1[0]>', 1, 32, 'import matplotlib.pyplot as plt\n', 1, 32)), <traceback object at 0x1202d7a40>) Note that this debug message make it looks like the I'm not sure why this is happening or why the code here is wrong. I'll investigate. |
Ha, I think it considers And that make me realize we should have a custom parser in |
They aren't actually that important for doctests because they are only used for the reporting, which we are bypassing anyways.
Doesn't do anything right now because success/failure isn't saved
In the future, do not force push to other people's branches. |
There seems to be a segfault from one of the plots in the
Although there's a separate question which is why the doctests are being run at all with |
I fixed the --no-exec flag. The segfault doesn't happen on main, though. I'm guessing it has something to do with with the fig managers. |
I think the culprit of I've pushed a commit that exclude just this function from being executed. I was also able to reproduce just with
|
papyri/gen.py
Outdated
doctests = doctest.DocTestParser().get_doctest( | ||
block, doctest_runner.globs, obj.__name__, filename, lineno | ||
) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
get_doctests also completely drop interleaving text, so we'll need to fallback to parse(...)
and create the DocTests object ourselves here.
This is more robust and correct than doing code.split('\n\n').
I've fixed the parser to be more robust for interleaving text. It now properly handles the case where text is right before an example. My main concerns now are if we are actually including everything we want in the output JSON. In particular, the JSON output doesn't include the prompts ( "example_section_data": {
"children": [
{
"type": "code",
"value": "x = np.arange(6)\ncondlist = [x<3, x>3]\nchoicelist = [x, x**2]\nnp.select(condlist, choicelist, 42)"
},
{
"type": "code",
"value": "condlist = [x<=4, x>3]\nchoicelist = [x, x**2]\nnp.select(condlist, choicelist, 55)"
}
] and in this branch "example_section_data": {
"children": [
{
"type": "code",
"value": "x = np.arange(6)\n"
},
{
"type": "code",
"value": "condlist = [x<3, x>3]\n"
},
{
"type": "code",
"value": "choicelist = [x, x**2]\n"
},
{
"type": "code",
"value": "np.select(condlist, choicelist, 42)\n"
},
{
"type": "text",
"value": "\n"
},
{
"type": "code",
"value": "condlist = [x<=4, x>3]\n"
},
{
"type": "code",
"value": "choicelist = [x, x**2]\n"
},
{
"type": "code",
"value": "np.select(condlist, choicelist, 55)\n"
}
], Compare the actual docstring:
Note the |
papyri/gen.py
Outdated
) | ||
) | ||
) | ||
figs.extend(figs) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
figs.extend(figs) | |
self.figs.extend(figs) |
I think, or no figures will be saved I belive.
@@ -41,3 +41,4 @@ exclude = [ "dask.utils:Dispatch", | |||
|
|||
#docs_path = "~/dev/dask/docs/source" | |||
exec_failure = 'fallback' | |||
execute_doctests = false |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You introduced this but are not using it, I'm guessing you intended something to exec
. I pushed a commit that rename all the usage of config.exec
to config.execute_doctests
for it to work as intended. You naming is better.
Ok, test are passing, let's merge and move on. |
This is incomplete but I'm going to try to deal with the following: 1) each line in a black example is after jupyter#308 it's own line, so try to collapse subsequent code blocks. 2) It seem that we get a number of report_failure, but failure is just when the output does not match, though when we have a block with multiple >>> in sequence and we ignore output on purpose they now are seen as failure. It's not super great and will need a buch of workaround
This is incomplete but I'm going to try to deal with the following: 1) each line in a black example is after jupyter#308 it's own line, so try to collapse subsequent code blocks. 2) It seem that we get a number of report_failure, but failure is just when the output does not match, though when we have a block with multiple >>> in sequence and we ignore output on purpose they now are seen as failure. It's not super great and will need a buch of workaround
Fixes #282
Still several todos here:
Here's an example:
Generates