search

gentzkow / template_archive

20 stars 36 forks source link

PR for #86: reformat template to fit tex compile #88

Closed ShiqiYang2022 closed 1 year ago

ShiqiYang2022 commented 1 year ago

Closes #86 .

This PR aim to review the reformat of template compile in #86. The purpose of this issue is described in https://github.com/gentzkow/template/issues/86#issue-1886725615. For this PR, I suggest the following steps:

Per conversation, @snairdesai has bandwidth to conduct the review. @snairdesai please flag any error or anything that might not be clear in any of the steps.

cc: @gentzkow @jc-cisneros

snairdesai commented 1 year ago

Update: Per conversation with @ShiqiYang2022, I have found a new conflict arising from the Mac M2 chip on my local machine which prevents compile here. Even the standard template is no longer compiling on my local machine (though it did previously) which suggests something was updated in the conda environment which is no longer supported by Apple Silicon.

A new issue will be opened to address this; in the meantime @jc-cisneros will take over testing here.

ShiqiYang2022 commented 1 year ago

@snairdesai Thanks for testing! I have requested @jc-cisneros to test this on his Macbook with intel chip.

While in the meantime I will open a new issue to address the inconsistency between different chips. Thanks for the great catch!

jc-cisneros commented 1 year ago

@ShiqiYang2022 if I try to re-run the .tex compile, I get this error:

*********************************************************
* Error with `run_latex`. Traceback can be found below. *
*********************************************************

Traceback (most recent call last):
  File "/Users/jccp/Desktop/template/paper_slides/../lib/gslab_make/gslab_make/run_program.py", line 283, in run_latex
    os.mkdir('latex_auxiliary_dir')
FileExistsError: [Errno 17] File exists: 'latex_auxiliary_dir'

Perhaps you could add a check that if this directory exists, then continue.

jc-cisneros commented 1 year ago

Confirming everything works as expected @ShiqiYang2022! Per our conversation, if the paper_slides module fails, the temporary directory will not be cleared and we will not be able to re-run the module until we erase it. Confirming this is intentional because that would provide us with intermediate output that we can use to fix any error.

ShiqiYang2022 commented 1 year ago

@jc-cisneros Thanks! This is because in your previous run, you did not populate the updated the gslab_make, and it got stuck since it cannot correctly refer the nested .tex file. When you manually killed the terminal, the session ended and latex_auxiliary_dir remained with intermediate output.

Per our conversation you did not delete this folder when you re-run .tex. And this latex_auxiliary_dir has the necessity for its existence, because it can help us check the intermediate outputs. I think all good here!

ShiqiYang2022 commented 1 year ago

The follow-up issue of https://github.com/gentzkow/template/pull/88#issuecomment-1769301802 is opened in #89.

ShiqiYang2022 commented 1 year ago

@jc-cisneros Given the problem described in https://github.com/gentzkow/template/pull/88#issuecomment-1769301802 is addressed in #89 #90, maybe it's a good time to give it another review, thanks!

ShiqiYang2022 commented 1 year ago

@gentzkow

We shifted template compile from .lyx to .tex in this PR and #86, and I am closing this PR and merging back.

Per #84(Comment), we want to compare .tex compile locally vs Overleaf and manage the difference properly. The difference can be properly managed by specifying the version of TexLive distribution consistent with that used in Overleaf, and (if co-author use other .tex distributions) use the packages that only are part of the standard TeX distribution. For details, please see https://github.com/gentzkow/template/issues/86#issuecomment-1768660960.

gentzkow commented 1 year ago

Great. Thanks!

Can we

  1. Make sure we add a /lyx/ directory to /extensions/ that has the Lyx version of paper_slides.

  2. Make sure our instructions for Overleaf include the note about how to make sure it's consistent w/ local compile.

ShiqiYang2022 commented 1 year ago

@gentzkow Thanks! Replies to this https://github.com/gentzkow/template/pull/88#issuecomment-1777498066:

  1. This has been done in 87f5afc. @jc-cisneros has cross-checked in this PR that we can complete the full run moving back .lyx, following the instructions of the ReadMe file.
  2. I just added that part into the instructions (ref: here).