Skip to content
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

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

Merged
merged 16 commits into from
Oct 24, 2023

Conversation

ShiqiYang2022
Copy link
Collaborator

@ShiqiYang2022 ShiqiYang2022 commented Oct 18, 2023

Closes #86 .

This PR aim to review the reformat of template compile in #86. The purpose of this issue is described in #86 (comment). 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

@ShiqiYang2022 ShiqiYang2022 linked an issue Oct 18, 2023 that may be closed by this pull request
3 tasks
@snairdesai
Copy link
Contributor

snairdesai commented Oct 18, 2023

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
Copy link
Collaborator Author

ShiqiYang2022 commented Oct 18, 2023

@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
Copy link
Contributor

@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
Copy link
Contributor

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
Copy link
Collaborator Author

@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
Copy link
Collaborator Author

The follow-up issue of #88 (comment) is opened in #89.

@ShiqiYang2022
Copy link
Collaborator Author

@jc-cisneros Given the problem described in #88 (comment) is addressed in #89 #90, maybe it's a good time to give it another review, thanks!

Copy link
Contributor

@jc-cisneros jc-cisneros left a comment

Choose a reason for hiding this comment

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

Thanks @ShiqiYang2022! My understanding is there were no changes in the code after my last review. Given that the only remaining lingering comment was addressed in a separate issue, this looks good to go.

@ShiqiYang2022
Copy link
Collaborator Author

@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 #86 (comment).

@gentzkow
Copy link
Owner

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
Copy link
Collaborator Author

ShiqiYang2022 commented Oct 24, 2023

@gentzkow Thanks! Replies to this #88 (comment):

  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).

@ShiqiYang2022 ShiqiYang2022 merged commit c5f5695 into master Oct 24, 2023
@ShiqiYang2022 ShiqiYang2022 deleted the 86-reformat-template-to-fit-tex-compile branch October 24, 2023 21:07
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.

Reformat template to fit .tex compile
4 participants