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

Allow >>> Python style doctests #236

Closed
wants to merge 1 commit into from
Closed

Allow >>> Python style doctests #236

wants to merge 1 commit into from

Conversation

skycaptain
Copy link

We're using Sphinx for generating HTML docs out of help texts. Sphinx recognizes doctests starting with >>> and formats them as accordingly. Maybe we could add support for that.

This would allow us to use:

function number = getMyNumber()
% Returns the number
%
%   Returns:
%       number (numeric): The number
%
%   Example:
%       >>> getMyNumber()
%       5
%
    number = 5;
end

Instead of:

function number = getMyNumber()
% Returns the number
%
%   Returns:
%       number (numeric): The number
%
%   Example:
%       ::
%
%           >> getMyNumber()
%           5
%
    number = 5;
end

@cbm755
Copy link
Collaborator

cbm755 commented Oct 1, 2019

I'm on fence (say "+0" to merge) and would like a little more information first. Thoughts:

  • >> is the actual Matlab/Octave GUI prompt so in my mind that's the preferred notation.
  • doctests are primarily documentation for users.
  • Is it difficult to configure Sphinx to recognize >> instead?
  • OTOH, flexibility is good and I don't see this as likely to cause problems. And we already suppose texinfo-style as well as >>.

@mtmiller
Copy link
Collaborator

mtmiller commented Oct 1, 2019

I think I would be ok with this change, but maybe change the regex to allow any number of >>>>>>> (2 or more) followed by a space to indicate a prompt?

I certainly don't think this should be advertised or encouraged, but it seems fairly harmless to allow as an option for those who want to exploit this magic shortcut.

I glanced at Sphinx and Docutils, and as far as I can tell, the prompt is hardcoded here, no option to teach Docutils to recognize a different prompt string. But again, this is just a convenience shortcut recognized by Sphinx for embedded Python doctests.

@@ -30,7 +30,7 @@
% extract tests from docstring
TEST_RE = [ % loosely based on Python 2.6 doctest.py, line 510
'(?m)(?-s)' ... % options
'(?:^ *>> )' ... % ">> "
'(?:^ *>>>? )' ... % ">> " or ">>> "
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What about (?:^ *>>>* )? This would make valid prompts out of >>, or >>>, or even >>>>>>, without giving any special significance to Python's default >>>.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We could do that, of course. Can you think of a situation, where it would be useful to have more than three >?

@skycaptain
Copy link
Author

skycaptain commented Oct 1, 2019

Doctests come from Python (to my best knowledge). Since Matlab does not have a mature documentation system, we are required to use s.th. like Sphinx instead. Afaik it is not possible to change that in Sphinx, since Python doctests are pretty straight-forward.

>> is the actual Matlab/Octave GUI prompt so in my mind that's the preferred notation.

That's right. But, as @mtmiller stated, I wouldn't advertise this, just add support for people, who are using Sphinx for document generation. That's why I only changed the regex searching for tests not the other occurances of >> , e.g. in error messages.

doctests are primarily documentation for users.

You're right. Readability is most important here. I don't expect users to care about two or three guillemet. I think they care more about the additional indentation level and two extra lines.

@cbm755
Copy link
Collaborator

cbm755 commented Oct 1, 2019

Ok, I'm convinced, with or without @mtmiller's regex.

@catch22 are you ok with this too?

@mtmiller
Copy link
Collaborator

mtmiller commented Oct 1, 2019

One possible counterargument to this: Octave, but not Matlab, uses the >>> marker when reporting syntax errors. So a valid Octave doctest block might be written like this

## >> x = 2 * (1 + 1))
## ??? parse error:
## 
##   syntax error
## 
## >>> x = 2 * (1 + 1))
##                    ^

Unfortunately this doesn't work with doctest right now, should it? Obviously recognizing >>> as a prompt would change this from one test into two tests.

@catch22
Copy link
Collaborator

catch22 commented Oct 2, 2019

Hmm, that's a good point @mtmiller. It is probably something that one could reasonably expect to work with doctest. The reason that it hasn't come up yet is that one would typically just match on the syntax error but add ... in place of the last line, so it wouldn't be terrible if we didn't support it. What do you think, @cbm755?

@skycaptain
Copy link
Author

Hmm, I think the issue with this example is different one. Python doctest is using the keyword <BLANKLINE> for empty lines. Doctests are then defined as one block of text, starting with >>> and no real empty lines. So your example would become:

## >> x = 2 * (1 + 1))
## ??? parse error:
## <BLANKLINE>
##   syntax error
## <BLANKLINE>
## >>> x = 2 * (1 + 1))
##
## >> % The next doctest

See https://docs.python.org/3.7/library/doctest.html#how-are-docstring-examples-recognized

@catch22
Copy link
Collaborator

catch22 commented Oct 2, 2019

I believe we are not using a special keyword for empty lines (unlike Python).

From a quick glance the regex part capturing the output does stop upon meeting >> (rather than >> ).

@cbm755
Copy link
Collaborator

cbm755 commented Oct 2, 2019

I filed #237 for the "quick glance" comment of @catch22.

But I'm not sure what we should do here. They way we handle errors is already a bit wonky. We prepend whatever Matlab/Octave give us with ??? (see doctest_format_exception.m).

We could prepend all lines of error messages with ???... Then (I think) we could fix #237 and accept this PR. But why are prepending with ??? at all?

@cbm755
Copy link
Collaborator

cbm755 commented Oct 2, 2019

I filed #238 for ??? discussions.

@cbm755
Copy link
Collaborator

cbm755 commented Oct 2, 2019

Turns out we've actually had much of this discussion already in #100 sigh.

Interestingly, @oheim claimed almost no one was using >> in octave and matlab. Symbolic was an exception but is no longer using it.

@catch22: do you use >>?

Perhaps Sphinx usage (with >>>) is the main thing that would drive Matlab use of Doctest.

@cbm755
Copy link
Collaborator

cbm755 commented Oct 2, 2019

What if the first >>>* string on the page defined the prompt? That is, we find it, then build the regex. I think this would allow us to fix #237 and also accept something like this PR... I'll see if I can flesh this out a little.

@catch22
Copy link
Collaborator

catch22 commented Oct 2, 2019

That would certainly be a clever solution! (Is it too clever? 😄)

@skycaptain - are you directly running Sphinx on your Matlab code, or are you doing some preprocessing/extraction first? Is there a GitHub repository where we can have a look at your work flow?

@skycaptain
Copy link
Author

@catch22: I'm using sphinx-contrib/matlabdomain for in addition to Sphinx. The project itself is private I fear. I'll try to setup a public repo mimicing my usecase.

@catch22
Copy link
Collaborator

catch22 commented Oct 2, 2019

In a way, it would make more sense to "fix" sphinx-contrib/matlabdomain so that it can deal with Matlab/Octave-style prompts than for us to use Python's prompt. Is this a realistic option?

On the other hand, even Octave itself isn't consistent in its prompt. I think it's >> in the GUI and octave:n> in the terminal. My main concerns about supporting >>> are rather:

  1. The issue that syntax errors often contain lines starting with >>>: One way to overcome this would be to prefix every line of the exception message by error: . Perhaps this is anyways how it should be?

  2. That we'll identify more and more discrepancies where Python's doctest format differs from idiomatic Octave/Matlab, including ones that we will not be able to "patch over".

@skycaptain
Copy link
Author

Is this a realistic option?

Hmm. Not an elegant solution, but one could Monkey-patch this line.

I filed sphinx-contrib/matlabdomain#94

@catch22
Copy link
Collaborator

catch22 commented Oct 10, 2019

Thanks for filing the issue at sphinx-contrib/matlabdomain#94. This sounds like a problem best solved by discussing across projects.

@cbm755 cbm755 changed the base branch from master to main January 3, 2023 21:16
@skycaptain skycaptain closed this by deleting the head repository Nov 8, 2023
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

Successfully merging this pull request may close these issues.

4 participants