Closed lboxell closed 7 years ago
Starting work on builder-related comments (pulling from #24, continually as PR gets pushed through)
@stanfordquan, FYI: I added a point in the comment above about using @gentzkow's suggestion on INPUT
from https://github.com/gslab-econ/template/issues/19#issuecomment-262024738.
Got it!
@stanfordquan, Per discussion in https://github.com/gslab-econ/template/issues/19#issuecomment-262325105, we want to do the following for logging:
Using a single log file (that replaces sconstruct.log and sconscript):
(1) We log only the builds scons runs, but we append rather than overwrite; so the log accumulates a record of every run. One can easily search to find the most recent run of a particular build.
I should be structured so that it is easy to search and filter to find a specific run. Run time for each build step along with warnings and errors should be printed to this file.
Progress: made target_file
vs log_file
change except for in build_stata()
and build_lyx()
(kept the same in lyx and postpone build_stata() for refactoring effort. Made change to check_code_extension()
. Made change in build_tables()
.
Also in progress: adding BadExecutableError for builders. Debating between using subprocess.check_output()
and then subprocess.CalledProcessError
vs. the current os.system()
call and a is_in_path()
logical check right before.
This is on hold until Dec 5 (target date for congress text release)
On hold while congress_text rebuild
Extension to the above build_lyx
error task:
Add specific lyx message to note that lyx build have failed. The error message when lyx build fail is
output/paper/MyPaper.pdf not found
which is rather cryptic. This error comes up when the shutil move
command to move the pdf file to the output folder fails (since the pdf file was never created, it was not found).
Working on this today.
Done! See template pull request and gslab-python pull request
Follow package organization established in #24.
and in misc.py
target_file
in the code makes it look like we require there to be a single target file. Really, we're only using this to pick a directory for the log file. So I might drop the lines starting withtarget_file
andtarget_dir
and instead sayShould we have all builder's return an informative error message for the case where the executable is not found / installed (so, e.g., if I call the Lyx builder and don't have lyx correctly on the path I get a nice error explaining what happened and suggesting that I make sure it's installed and on the path)? This is a very common case when people try to run our code, and it would be nice to handle it explicitly.
In build tables we can replace
source[1:len(source)]
withsource[1:]
I think we should restructure build_stata. I would suggest we define two custom functions get_stata_executable() and get_stata_options() so that the main builder ends with
and is otherwise the same as the other builders. Then we could structure get_stata_executable() as roughly:
This was code that was living in large_template and missed getting integrated into the original gslab_scons release. This step should involve removing this code from the template.
INPUT
from https://github.com/gslab-econ/template/issues/19#issuecomment-262024738.INPUT
suggestion, we should require that source have a single element, since these builders are designed to only handle a single source file?I should be structured so that it is easy to search and filter to find a specific run. Run time for each build step along with warnings and errors should be printed to this file.