Skip to content

Should we have a 'silent' cell magic? #16

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

Open
ideabucket opened this issue Oct 8, 2022 · 18 comments
Open

Should we have a 'silent' cell magic? #16

ideabucket opened this issue Oct 8, 2022 · 18 comments

Comments

@ideabucket
Copy link
Contributor

I was thinking about #4 again in light of PR #15 and trying to work out when the echo-all behaviour is most annoying. For me, it's usually when I want to avoid the screen filling up with a bunch of echoed commands that have little or no output. For example, if I'm putting together an etable it looks something like this:

quietly regress…
estimates store…

quietly regress…
estimates store…

…

etable…

Or in a setup block—since pystata doesn't pick up profile.do, every notebook I make starts with:

version 17
set varabbrev off
global data_root…
…

and other throat-clearing.

So I was wondering: what if we just added a cell magic, e.g. *%silent, that tells the kernel not to print the output of that particular cell unless an exception is thrown? I'm happy to take a crack at implementing this but I thought I'd put it up for discussion first.

@hugetim, would this be any good for your use case?

@ticoneva
Copy link
Owner

ticoneva commented Oct 8, 2022

That's a good idea. This can be done by passing quietly=True to pystata.stata.run().

For consistency with Stata, maybe we should name this magic *%quietly?

There is one question I have to ask though: why would you not use Stata's native quietly? Like

qui {
...
}

@ideabucket
Copy link
Contributor Author

ideabucket commented Oct 8, 2022

Primarily, because with a native quietly { … } block you still get the first line echoed back, and the empty dot prompt after it:

quietly {
    dosomething
    dosomethingelse
}
. quietly {

. 

I don't know why pystata behaves this way - if you type or paste a multiline quietly { … } block into Stata itself, you get the whole block in the results window, which makes sense: quietly suppresses the output of the command(s). (And this is consistent with what happens if you just run quietly dosomething.)

The theory behind this magic, on the other hand, is to suppress everything: the command echo and the output. That's also why I suggested a different name than 'quietly'; it's for a different purpose that doesn't really make sense outside the notebook environment. (But I'm not wedded to that.)

The less important reason for a magic over a quietly { … } block is that it doesn't require wrapping the cell in braces or indenting everything, which means less hassle and, if the notebook gets exported to a do-file, tidier code.

@hugetim
Copy link
Contributor

hugetim commented Oct 8, 2022

That would help for my use case, yes. And, to me, the *%quietly name makes some sense because, in a notebook, you see the commands in the block, akin to how the commands would show up in the log using a quietly block in Stata normally.

I think I'd still like to also have a *%no_echo magic that displays only the output of the commands as in #15 (with documented caveats about program definitions not working, limited allowance for #delimit, and any other issues that arise over time).

@ticoneva
Copy link
Owner

ticoneva commented Oct 9, 2022

Given the interest, I have added the logic necessary to implement such magics over the weekend. There are two new magics and a new configuration option:

Magics

  • *%noecho : suppress echo in current cell
  • *%quietly: suppress all output from current cell

Configuration

  • echo = None: the kernel will not echo any command

The logic should be able to handle multi-line comments and change of delimiter correctly. Please check and see if I miss anything.

@hugetim
Copy link
Contributor

hugetim commented Oct 9, 2022

I'm not sure I'm going to be able to take the time soon to understand how you're handling the delimiters. (One thing I was worried about with the quietly-noisily approach was handling the /// approach to multi-line commands--or weirder things like delimiter changes via local macros.) But I should be able to test it out a bit tomorrow.

@ticoneva
Copy link
Owner

ticoneva commented Oct 10, 2022

The way it works right now is, whenever a user requests echo supression, helpers.clean_code() is called to do two things:

  1. Recursively detect and apply the correct delimiter.
  2. Strip multi-line comments (/// and /* */).

The processed code is then passed to pystata.stata.run().

Following your comment, I have run extensive tests on #delimit's behavior.

  • I initially through you could specify the delimiter through macro, but it turns out that is not the case.
  • The way it actually works is, #delimit x changes the delimiter to ';' as long as x is not 'cr'.
  • In particular, it appears impossible to de-reference globals and locals to their values. E.g.
    global de cr
    #delimit "$de"
    
    would get you
    delimiter now ;
    
    I have tried all sorts of quotes and they make no difference.

@ideabucket
Copy link
Contributor Author

The way it actually works is, #delimit x changes the delimiter to ';' as long as x is not 'cr'.

Just to note that the Stata help file for delimit is quite explicit that it only supports two values: ; and cr.

@hugetim
Copy link
Contributor

hugetim commented Oct 10, 2022

Ok, that makes sense and sounds quite robust. Thank you!

What I actually had in mind with my macro-delimit comment, though, was something more along these lines:

local my_delimit "#delimit ;"
`my_delimit'

But I've never heard of anyone doing that, and I haven't tested it yet.

p.s. I see now from the delimit help file that it's not possible to set the delimiter in a previous notebook block and have it carry over to the next block, which was another thing I had been puzzled about.

@hugetim
Copy link
Contributor

hugetim commented Oct 10, 2022

Hmm, when defining programs (with echo = None), it will add noisily to each line within the program, which seems undesirable. Maybe we should prevent that--treat lines between a program define and end differently?

update: Actually, program definition doesn't work in that setting (echo = None or with *%noecho). I get:

SystemError:   ...
  5. noisily end
  6. 
unexpected end of file
r(612);

update2: The same problem occurs for something like a for loop. It complains about the noisily } line. But maybe exceptions for lines that start with } or end are sufficient?

@hugetim
Copy link
Contributor

hugetim commented Oct 10, 2022

With echo = None, the *%quietly magic isn't working as I expected. It seems the noecho behavior is preempting it.

@ticoneva
Copy link
Owner

The way it actually works is, #delimit x changes the delimiter to ';' as long as x is not 'cr'.

Just to note that the Stata help file for delimit is quite explicit that it only supports two values: ; and cr.

Yes, which is why I thought the only acceptable values after #delimit are ';' and 'cr'. Turns out that's not the case.

@ticoneva
Copy link
Owner

ticoneva commented Oct 11, 2022

p.s. I see now from the delimit help file that it's not possible to set the delimiter in a previous notebook block and have it carry over to the next block, which was another thing I had been puzzled about.

In Stata the effect of #delimit ; does not extend beyond the current block of code being run—e.g. if you highlight a block of code with #delimit ; in it, run it, and then run another block of highlighted code, the latter won't honor the delimiter change.

Since pystata-kernel process the delimiter before passing the code to Stata, I can actually make the effect persistent across notebook cells. It's arguable whether doing so is more in line with native Stata behavior or not: is running a cell more akin or running a block of highlighted code in a do file, or should we treat the execution of consecutive cells as one single run? I am open to either interpretration. I could also make this into an option.

Maybe we should prevent that--treat lines between a program define and end differently?

update2: The same problem occurs for something like a for loop. It complains about the noisily } line. But maybe exceptions for lines that start with } or end are sufficient?

Thanks for the catch. I will post a commit on that soon.

@ticoneva
Copy link
Owner

Program definition and loops should now be properly excluded. The quietly magic should also work under the echo = None configuration.

@hugetim
Copy link
Contributor

hugetim commented Oct 11, 2022

is running a cell more akin or running a block of highlighted code in a do file, or should we treat the execution of consecutive cells as one single run?

As I think about it, a cell feels more akin to running a block of highlighted code. So I'd vote for keeping the default as is (and I never use delimit myself, anyhow). But an option would make sense if there's demand for it.

@hugetim
Copy link
Contributor

hugetim commented Oct 11, 2022

The quietly magic is working for me, but I'm running into various other issues as I test out the noecho mode. For example, the commands within a loop would be echoed. I started #18 to address that and a few other niggles.

@hugetim
Copy link
Contributor

hugetim commented Oct 11, 2022

I'm also hitting a corner-case delimit error when #delimit cr is on the last line of code in a cell. (I now understand that there's never a need for such a #delimit at the end of a block, but it is valid Stata code.) For instance:

#delimit ;
disp "hello";
#delimit cr

triggers:

command cr is unrecognized
r(199);

I haven't been able to find a fix so far, myself.

@hugetim
Copy link
Contributor

hugetim commented Oct 11, 2022

As I think about it, a cell feels more akin to running a block of highlighted code. So I'd vote for keeping the default as is (and I never use delimit myself, anyhow). But an option would make sense if there's demand for it.

Hmmm, resetting delimit to cr at the top of every cell prevents the exported do file from working. That makes me think the default should be to make the effect persistent across notebook cells (unless we can insert any needed #delimit crs into the exported script).

@hugetim
Copy link
Contributor

hugetim commented Nov 22, 2022

Hmmm, resetting delimit to cr at the top of every cell prevents the exported do file from working. That makes me think the default should be to make the effect persistent across notebook cells

To follow up, I've now made #delimit; persist across cells (and also addressed the other issues raised in the above discussion) in the latest release of https://github.com/hugetim/nbstata

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

3 participants