Skip to content

Add timer and "no errors" reporting#63

Merged
ankritisingh merged 43 commits into
devfrom
reprun-46-stable
Jul 29, 2024
Merged

Add timer and "no errors" reporting#63
ankritisingh merged 43 commits into
devfrom
reprun-46-stable

Conversation

@ankritisingh
Copy link
Copy Markdown
Contributor

  1. Added a time which is displayed in seconds if less than a minute, and in minutes otherwise.

In seconds:

image

In minutes:

image

  1. For stable results:
    It displays a line if the table is empty:

image

.
However, if there were any changes reported in the "main.do" file, the line "no mismatches" does not appear when "stepping back into the file." I couldn't determine how to include this, but I believe it is fine (for now at least) since changes were reported at least once in that file. See below:

image

Comment thread src/ado/reprun.ado Outdated
Comment on lines +10 to +11
timer clear 1
timer on 1
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I would suggest using a higher number -- if the user is using a timer, they may use 1 and this will cause an error or other collision.

Comment thread src/ado/reprun.ado Outdated
Comment on lines +158 to +169
if `r(t1)' > 60 {
local duration : di %9.3f `r(t1)'/60
noi di as res ""
noi di as res `"{phang}Total run time: `duration' minutes {p_end}"'
}

else {
local duration : di %9.3f `r(t1)'
noi di as res ""
noi di as res `"{phang}Total run time: `duration' seconds {p_end}"'
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

%9.2f is probably sufficient, but you may instead consider reporting MM:SS when it runs over 60 seconds; and HH:MM:SS when it runs over 60 minutes, as this is more standard notation and easier for the user to interpret. Reporting days is probably unnecessary.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Updated it so now it is displayed as:

  1. Seconds:
    Screenshot 2024-07-17 at 7 46 53 PM

  2. Minutes:
    Screenshot 2024-07-17 at 7 49 02 PM

  3. Hours:
    Screenshot 2024-07-17 at 7 46 21 PM

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Nice!

Comment thread src/ado/reprun.ado Outdated
local prev_line2 ""

* Local for empty tables
local any_lines_written 0
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

You might be able to find a way to reset this when "stepping back"; alternatively, you might not want to have this show up until each file is closed for the last time. This may or may not be predictable behavior.

I would suggest, since we will know what line the file has called a sub-do-file at, to report "no differences/errors in lines XX-YY" (don't worry about the exact message wording yet). I think we should be able to pick up XX/YY? @kbjarkefur?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I have not been able to figure this out. If there are no changes in the main file at all, then it does display "no mismatches...". But I have not been able to figure out how to reset it when "stepping back".

Can think about this more after working on other things for the conference.

@ankritisingh
Copy link
Copy Markdown
Contributor Author

@bbdaniels Updated the help file here for your review.

@ankritisingh ankritisingh requested a review from bbdaniels July 19, 2024 18:18
@bbdaniels
Copy link
Copy Markdown
Contributor

bbdaniels commented Jul 19, 2024

@bbdaniels Updated the help file here for your review.

Great start! The example should be modified a little bit. I would not include bysort here since we are still working on handling that functionality correctly -- just use sort and gen directly.

I would also split into two examples -- one where an issue is due to the lack of a seed, and one where it is due to an unstable sort.

We should then include the exact output from reprun so the user can see it for all settings, and also see how early mismatches cause later ones. Then, for each of these two examples, show the resolution.

Similarly, I would expand the example of a recursive call to show that functionality -- perhaps put a sorting issue in one file and a seeding issue in another, and show how solving them top-to-bottom works (and perhaps put in a cascading issue where solving bottom-to-top does not produce a solution).

@ankritisingh
Copy link
Copy Markdown
Contributor Author

@bbdaniels Updated the help file here for your review.

Great start! The example should be modified a little bit. I would not include bysort here since we are still working on handling that functionality correctly -- just use sort and gen directly.

I would also split into two examples -- one where an issue is due to the lack of a seed, and one where it is due to an unstable sort.

We should then include the exact output from reprun so the user can see it for all settings, and also see how early mismatches cause later ones. Then, for each of these two examples, show the resolution.

Similarly, I would expand the example of a recursive call to show that functionality -- perhaps put a sorting issue in one file and a seeding issue in another, and show how solving them top-to-bottom works (and perhaps put in a cascading issue where solving bottom-to-top does not produce a solution).

Okay, thanks! I will update the examples.
I tried adding the results, but since we are using ad_sthlp, they are not being displayed properly in .md version and then won't display in the smcl version too.
I will try again though.

@ankritisingh
Copy link
Copy Markdown
Contributor Author

@bbdaniels Updated the help file here for your review.

Great start! The example should be modified a little bit. I would not include bysort here since we are still working on handling that functionality correctly -- just use sort and gen directly.
I would also split into two examples -- one where an issue is due to the lack of a seed, and one where it is due to an unstable sort.
We should then include the exact output from reprun so the user can see it for all settings, and also see how early mismatches cause later ones. Then, for each of these two examples, show the resolution.
Similarly, I would expand the example of a recursive call to show that functionality -- perhaps put a sorting issue in one file and a seeding issue in another, and show how solving them top-to-bottom works (and perhaps put in a cascading issue where solving bottom-to-top does not produce a solution).

Okay, thanks! I will update the examples. I tried adding the results, but since we are using ad_sthlp, they are not being displayed properly in .md version and then won't display in the smcl version too. I will try again though.

@bbdaniels I updated the examples, splitting into two for clarity and reusing content for main.do to keep things concise. I tried to include the result, but the tables don't render well in Markdown or SMCL due to issues with merging cells. It seems that using ad_sthlp isn't a viable option here.

Maybe @kbjarkefur knows how to do it with ad_sthlp

Alternatively, we could link to a Dime Wiki article, as we've done for other commands. This would allow us to include screenshots and other details. I can work on this after the Stata conference.

@ankritisingh ankritisingh mentioned this pull request Jul 28, 2024
23 tasks
@bbdaniels bbdaniels changed the title For issue #10 #49 Add timer and "no errors" reporting Jul 29, 2024
@bbdaniels bbdaniels changed the base branch from reprun-46 to dev July 29, 2024 14:25
@bbdaniels
Copy link
Copy Markdown
Contributor

Retargeting to dev so we can finish after conference

@ankritisingh ankritisingh merged commit a6b922b into dev Jul 29, 2024
@ankritisingh ankritisingh deleted the reprun-46-stable branch July 30, 2024 18:18
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.

reprun: add line for stable results (empty table) reprun: Include timer for entire run

2 participants