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

eqctier3-6cea0c38-0a26-4b6e-8673-d8349bf10b9d #102

Merged
merged 18 commits into from
Sep 20, 2024

Conversation

malmans2
Copy link
Member

@malmans2 malmans2 commented Jun 6, 2024

Closes #47
Closes #48
Closes #91
Closes #92
Closes #95
Closes #96

Copy link

github-actions bot commented Jul 29, 2024

PR Preview Action v1.4.8
🚀 Deployed preview to https://ecmwf-projects.github.io/c3s2-eqc-quality-assessment/pr-preview/pr-102/
on branch gh-pages at 2024-09-19 16:50 UTC

@ecmwf-projects ecmwf-projects deleted a comment from github-actions bot Jul 29, 2024
@crgoddard
Copy link
Collaborator

@malmans2 I have added subfolders for CMIP6 and CORDEX to avoid the need to use these in the titles (which is not very user friendly in the sidebar or section TOC). Please check if there are any implications for permalinks, or something I haven't foreseen, before the squash and merge.

Note that we will also want to activate the subsections for ECVs, but that will be easier as the .ipynbs are already in subfolders (but we do want to split Land_Hydrology&Cryosphere_ECVs into two sections).

@malmans2
Copy link
Member Author

We should make changes like this in separate PRs, merge them into the main branch, and then merge the updated main into the other PRs. This is important because the changes affect the repo structure and impact all notebooks (in this case, all CMIP6 and CORDEX notebooks).

All climate projection permalinks and previews will be affected, as the subdirectory is part of the URL. Therefore, these notebooks should not be approved until the new permalink is shared in the CIM.

I'll open a separate PR and ping you there for review.

@crgoddard
Copy link
Collaborator

Ah okay, I guess I misunderstood the 'perma' in permalink then. So after this is merged to main, a link like this will break? 'https://github.com/ecmwf-projects/c3s2-eqc-quality-assessment/blob/61ed6d347dc7aa7646d5c98764979a94a0aa8c01/Climate_Projections/climate_projections-cmip6_climate-and-weather-extremes_q02.ipynb'

On the implications for CORDEX, that does make sense. I was planning to merge the changes from main (adding CMIP6, and the subfolders) into the CORDEX branch when I work on that. For now I will only push the other changes to the CORDEX nbs (fixing icons, headings etc) and not change the folder structure.

@malmans2
Copy link
Member Author

That permalink won't break and will remain functional.

However, I'm worried about the Jupyter Book URL. If I understand correctly, the CIM infers the Jupyter Book URL from the permalink, and that's the URL displayed in the web portal. If we change the filename or location of the file, the Jupyter Book URL will also change. Therefore, if the notebook is approved before this change is communicated in the CIM, the web portal will end up with a broken URL.

@crgoddard
Copy link
Collaborator

The inference is not in place yet, so we manually add the book link in the CIM at the final approval step - so for now it will not be an issue!

@malmans2
Copy link
Member Author

OK, so no problem if the change is made before the final approval step.

@crgoddard
Copy link
Collaborator

Waiting for the outcome of the splitting PR before the merge

@crgoddard crgoddard merged commit f8b0d9e into main Sep 20, 2024
3 checks passed
@crgoddard crgoddard deleted the eqctier3-6cea0c38-0a26-4b6e-8673-d8349bf10b9d branch September 20, 2024 06:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment