QuantEcon / lecture-python.myst

Quantitative Economics with Python
https://python.quantecon.org
84 stars 44 forks source link

Remove Migrated Lecture `ar1_processes` #385

Closed HumphreyYang closed 9 months ago

HumphreyYang commented 9 months ago

Hi @mmcky,

This PR removes lectures that have been migrated to the intro series (https://github.com/QuantEcon/lecture-python-intro/issues/334).

The AR(1) lecture is now in lecture-python-intro/pull/335

Do we still have a template for the migrated lecture? Apologize that I cannot find the template anymore in the commit thread.

mmcky commented 9 months ago

Do we still have a template for the migrated lecture? Apologize that I cannot find the template anymore in the commit thread.

@HumphreyYang I am not sure what you mean by template? (a process?)

We will want to also setup a redirect using sphinx-reredirect

mmcky commented 9 months ago

https://documatt.com/sphinx-reredirects/

HumphreyYang commented 9 months ago

Do we still have a template for the migrated lecture? Apologize that I cannot find the template anymore in the commit thread.

@HumphreyYang I am not sure what you mean by template? (a process?)

I remembered that we have a small admonition at the top that says the lecture has been migrated and will be taken down after a month. Please correct me if I were wrong.

mmcky commented 9 months ago

Next Steps

Once the new locations are live and published

HumphreyYang commented 9 months ago

Do we still have a template for the migrated lecture? Apologize that I cannot find the template anymore in the commit thread.

@HumphreyYang I am not sure what you mean by template? (a process?)

I remembered that we have a small admonition at the top that says the lecture has been migrated and will be taken down after a month. Please correct me if I were wrong.

I found it for the heavy_tails lecture. I will put this in place now.

mmcky commented 9 months ago

thanks @HumphreyYang.

I guess we need to choose if we redirect automatically to the new location OR use that admonition to indicate the lecture has moved. Let's go with the admonition for now (as that is what we have done in the past) and think about the redirect process. We could add the admonition to the top of the new location (for a month instead) to say this lecture location has changed and this is the new location.

mmcky commented 9 months ago

@HumphreyYang

We could add the admonition to the top of the new location (for a month instead) to say this lecture location has changed and this is the new location.

Actually - let's give this a go.

We will need to:

HumphreyYang commented 9 months ago

Many thanks @mmcky,

I think it is a better design choice. Readers cannot see the migration notice for the heavy_tail lecture as they are redirected to the new page : )

mmcky commented 9 months ago

Readers cannot see the migration notice for the heavy_tail lecture as they are redirected to the new page : )

We couldn't have both as the md file can't exist when setting up the redirect. So I think the admonition in the new location is the better option. You end up in the right spot and then we let them know the location is new. I think that works well. 👍

HumphreyYang commented 9 months ago

scalar_dynam has been migrated (@HumphreyYang were you able to diff them to make sure all changes were transferred?)

Many thanks @mmcky,

I just ran a diff and found one section is deleted in the intro version. Perhaps we should leave it for now, and I will consult John in our next meeting.

HumphreyYang commented 9 months ago

The warnings are complaining about the missing ar1_processes in the intro series.

We should get green ticks once ar1_processes is merged and live in the intro series.

mmcky commented 9 months ago

@HumphreyYang this lecture is now live

https://intro.quantecon.org/ar1_processes.html

mmcky commented 9 months ago

@HumphreyYang I have added the redirect for the ar1_processes lecture to https://intro.quantecon.org/ar1_processes.html

github-actions[bot] commented 9 months ago

🚀 Deployed on https://65b331b9e3fde16f8cba5402--nostalgic-wright-5fa355.netlify.app

mmcky commented 9 months ago

@HumphreyYang well this is interesting. Are these the warnings to showed up last time?

/__w/lecture-python.myst/lecture-python.myst/lectures/finite_markov.md:677: WARNING: unknown document: intro:ar1_processes
/__w/lecture-python.myst/lecture-python.myst/lectures/inventory_dynamics.md:286: WARNING: unknown document: intro:ar1_processes
/__w/lecture-python.myst/lecture-python.myst/lectures/linear_models.md:47: WARNING: unknown document: intro:ar1_processes
/__w/lecture-python.myst/lecture-python.myst/lectures/kesten_processes.md:41: WARNING: unknown document: intro:ar1_processes
/__w/lecture-python.myst/lecture-python.myst/lectures/kesten_processes.md:173: WARNING: unknown document: intro:ar1_processes
HumphreyYang commented 9 months ago

Many thanks @mmcky,

I fixed them in https://github.com/QuantEcon/lecture-python.myst/pull/385/commits/646cf0b076468b73a68c89865b0fbccedc9efe04

and we have green ticks after that.

Perhaps redirects overrules intersphinx? We can try deleting intersphinx tags at the front.

mmcky commented 9 months ago

Many thanks @mmcky,

I fixed them in 646cf0b

and we have green ticks after that.

Perhaps redirects overrules intersphinx? We can try deleting intersphinx tags at the front.

thanks @HumphreyYang yeah there definitely seems to be an issue using both intersphinx and reredirects.

I know inter sphinx doesn't do connections to external website (at the page level) so I will take a closer look at https://documatt.com/sphinx-reredirects/usage.html to see if perhaps we should use these for internal links as well?

I am surprised though as inter sphinx is using a domain so should be independent. Will have to figure this one out.

HumphreyYang commented 9 months ago

Many thanks @mmcky,

I experimented removing the tag or adding an empty ar1_processes but no luck.

I was also thinking about its implications on our new series. I think we can prioritize intersphinx in our new lecture series as we might be able to create an empty repository (with only the config file) hosting the redirect mappings between old intermediate series and new series. In this case, we do not have internal intersphinx references in the redirect repository, and intersphinx will also work in our new series.

mmcky commented 9 months ago

I think we can prioritize intersphinx in our new lecture series as we might be able to create an empty repository (with only the config file) hosting the redirect mappings between old intermediate series and new series. In this case, we do not have internal intersphinx references in the redirect repository, and intersphinx will also work in our new series.

Thanks @HumphreyYang I don't fully understand what you are proposing. Can you explain?

We will need page redirection support to make sure anyone with python.quantecon.org/ar1_processes ends up at intro.quantecon.org/ar1_processes. This is important as 25K a month will be making use of the old addresses until they update.

HumphreyYang commented 9 months ago

Many thanks @mmcky

I am not sure whether it will work but I am proposing to have a fresh repository with old intermediate series URL. It will host the redirect config and intro page only (without lectures). In this case, it wouldn’t conflict with intersphinx since lectures are hosted in our target repositories.

Please let me know your thoughts on this.

mmcky commented 9 months ago

@HumphreyYang are you replicating this locally -- I can't so the mystery depends. I have triggered a new build run to confirm.

HumphreyYang commented 9 months ago

@HumphreyYang are you replicating this locally -- I can't so the mystery depends. I have triggered a new build run to confirm.

Yes, I can replicate this locally. I will have another go today.

mmcky commented 9 months ago

@HumphreyYang I am definitely getting no local build errors for either html or pdflatex using a fresh environment of the base environment.yml. It would be helpful if you can confirm. I am going to do a test on this PR by removing the reredirect setting to see if this changes -- then will restore.

mmcky commented 9 months ago

@HumphreyYang the error still persists without ca5a20a so maybe intersphinx isn't working as it should?

HumphreyYang commented 9 months ago

Hi @mmcky,

This is getting more interesting as 646cf0b passed, and the only change afterwards was adding redirects. I am running a fresh build locally and will report my results once they are available.

mmcky commented 9 months ago

@HumphreyYang I think I have figure this out 🤞 -- I think the settings are misspecified in the context of the yml file. I'll push a test commit in a minute or two.

mmcky commented 9 months ago

@HumphreyYang I am rebuilding the cache to check there isn't some form of corruption.

mmcky commented 9 months ago

@HumphreyYang no idea why cdf81e8 built OK when the cache hasn't finihsed rebuilding yet. Clearly some instability. I will re-run this once cache is finalised to check all is green after that.

mmcky commented 9 months ago

@HumphreyYang I think this is working nicely now.