Implement echo=None functionality via run-as-program - Githubissues

ticoneva / pystata-kernel

Jupyter kernel for Stata based on pystata

GNU General Public License v3.0

13 stars 6 forks source link

Implement echo=None functionality via run-as-program #21

Closed hugetim closed 1 year ago

hugetim commented 2 years ago

Addresses #16, supersedes #15 and #18.

hugetim commented 2 years ago

I'm not currently aware of any valid Stata code this can't handle. In particular, #delimit statements actually seem to be OK within program definitions, as far as I can tell (and it's no longer a program for my run-as-program code due to your clean_code converting the delimiters). But there are two output quirks:

The run-as-program workaround generates a blank line in the output (for each multi-line block of code it is used for).
Multi-line mata programs are fully echoed. Otherwise, it seems they would show no output at all.

hugetim commented 1 year ago

Just to follow up on this, do you have an estimate on when you might have time to review this pull request? I'm looking to roll this out to others in my organization, and I'm wondering whether I should have them install from my own fork or wait to do it more cleanly via pip install pystata-kernel. Thanks! (I really appreciate your volunteered efforts here and sincerely don't want to come across as demanding or rushing you.)

hugetim commented 1 year ago

For anyone who sees this, I have now created a separate project which implements this pull request (after reorganizing the code) and makes other improvements: https://github.com/hugetim/nbstata

ticoneva commented 1 year ago

My apology for being absent for so long. Sickness and work have kept me from coming online. I have merged your request. I am also thinking of how to make this project sustainable so that one person's sickness will not bring updates to a halt. I could perhaps add you as a collaborator?

hugetim commented 1 year ago

Thanks! I would like to collaborate but I'm not sure the best way to go about it. I think I've made a number of improvements to my version in the past week or so. But they would be hard to port over to pystata_kernel because I've ended up doing a lot of refactoring, as well as restructuring the code to work as an nbdev project (something I've been wanting to try out for a while). Would you like to be a collaborator on nbstata? Or what do you think?

hugetim commented 1 year ago

Maybe we should arrange a video call to talk about this?

On Mon, Nov 28, 2022, 3:32 PM Vinci Chow @.***> wrote:

Merged #21 https://github.com/ticoneva/pystata-kernel/pull/21 into main.

— Reply to this email directly, view it on GitHub https://github.com/ticoneva/pystata-kernel/pull/21#event-7906999499, or unsubscribe https://github.com/notifications/unsubscribe-auth/AB63GLG642B2VQ5RIMA43CTWKUQFRANCNFSM6AAAAAARDQKB2Q . You are receiving this because you authored the thread.Message ID: @.***>

hugetim commented 1 year ago

To be more specific about the fixes and features I've added in nbstata, looking back:

Made #delimit persist across cells
Added some automatic tests, with Github Actions CI, using nbdev. (Only stuff that doesn't use Stata can be tested in the cloud, but I've also added some semi-automatic Stata tests in the code notebooks that I can run locally.)
Clarified the install instructions
Improved _run_as_program (which originated in this pull request) to remove the extra line and clean itself up properly in case of a Stata code error
Added an echo magic
Refined clean_code (renamed to standardize_code) to handle some edge cases that came up during testing

I've been super-fortunate to have some free time at my job recently during which I've been able to work on this stuff on the clock. But that is unlikely to last for long.