Added a link checker step in the doctests jobs using both Lychee and …

[RohitP2005]

Sphinx Link Checker Integration in CI

Description

This PR introduces a fix to run the Sphinx link checker in parallel with Lychee during the CI pipeline. This resolves the issue of missing or broken links not being detected while ensuring proper integration with the existing workflow. The Sphinx link checker builder has been added to the CI process to ensure links are checked effectively, with broken links automatically identified and flagged.

Fixes: #3387

---

Type of change

• New feature (non-breaking change that adds functionality)
• Optimization (back-end change that speeds up the code)
• Bug fix (non-breaking change that fixes an issue)

---

Key checklist:

• No style issues: $ pre-commit run (or $ nox -s pre-commit) (see CONTRIBUTING.md for how to set this up to run automatically when committing locally)
• All tests pass: $ python -m pytest (or $ nox -s tests)
• The documentation builds: $ python -m pytest --doctest-plus src (or $ nox -s doctests)

---

Further checks:

• Code is commented, particularly in hard-to-understand areas
• Tests added that prove fix is effective or that feature works

---

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 99.22%. Comparing base (a7253b8) to head (f955f5d).
Report is 3 commits behind head on develop.

Additional details and impacted files

@@           Coverage Diff            @@
##           develop    #4686   +/-   ##
========================================
  Coverage    99.22%   99.22%           
========================================
  Files          303      303           
  Lines        23070    23077    +7     
========================================
+ Hits         22891    22898    +7     
  Misses         179      179           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[kratman]

I still think this should be part of the doctests or docs workflow instead of with lychee. Lychee is really unstable so most people will not even look at the failure

[RohitP2005]

I still think this should be part of the doctests or docs workflow instead of with lychee. Lychee is really unstable so most people will not even look at the failure

can u kindly be specific , cause it was asked to run Lychee and and Sphinx parallelly

[agriyakhetarpal]

Sure, what Eric meant was to move the job you added + the current docs-related tests in test_on_push.yml to a separate workflow file (the latter part of my previous comment)? To avoid building the docs thrice, we can build them without the nox sessions because a CI runner is isolated enough.

[RohitP2005]

Sure, what Eric meant was to move the job you added + the current docs-related tests in test_on_push.yml to a separate workflow file (the latter part of my previous comment)? To avoid building the docs thrice, we can build them without the nox sessions because a CI runner is isolated enough.

Understood @agriyakhetarpal
Thanks for the clarification
I have made the changes u requested , kindly update me for further changes required .

[agriyakhetarpal]

Sorry – I think some of my requests for changes might have overlapped with Eric's. Please ignore the ones that have already been done.

[kratman]

@RohitP2005 Please do not force push after the review has started. It makes it extremely hard to track changes

[RohitP2005]

@RohitP2005 Please do not force push after the review has started. It makes it extremely hard to track changes

Sorry for the trouble eric, I will make a commit including the changes as u requested

[RohitP2005]

Changes I Made:

• Added lychee_url_checker.yml — requested by Eric
• Removed the commented lines from test_on_push.yml — requested by @agriyakhetarpal
• Removed env and the respective token — requested by @agriyakhetarpal and Eric
• Renamed docs.yml to test_docs_build.yml — requested by Eric
• Removed the lychee job from the test_docs_build.yml — requested by Eric
• Removed unnecessary comments — requested by @agriyakhetarpal

I hope this satisfies the changes u both requested, apologies as i was confused with instruction given earlier

[agriyakhetarpal]

Just two more changes, thanks for taking this up, @RohitP2005!

[agriyakhetarpal]

@all-contributors please add @RohitP2005 for infra

[allcontributors]

@agriyakhetarpal

I've put up a pull request to add @RohitP2005! 🎉

[agriyakhetarpal]

Please feel free to fix the failing links in this PR or in a quick follow-up, I have no preference.

[kratman]

Looking at the failures, it looks like there might be some stuff to fix before we start using this:

(source/user_guide/installation/gnu-linux-mac: line   87) broken    index.html#install-required-dependencies - 
(source/user_guide/installation/gnu-linux-mac: line  106) broken    index.html#installation - 

It seemed like those links worked when I tried them.

Some of the research paper links fail:

(source/api/models/lithium_ion/dfn: line   16) broken    https://www.sciencedirect.com/science/article/pii/S0378775322001604 - 403 Client Error: Forbidden for url: https://www.sciencedirect.com/science/article/pii/S0378775322001604

We should probably have fixes in place for those before we merge this, otherwise all PRs will be marked as failing

[agriyakhetarpal]

Yeah, just saw the first ones as well, unsure how to fix those... In the meantime, we ignore all links from ScienceDirect with Lychee, we can use a similar option in conf.py: https://www.sphinx-doc.org/en/master/usage/configuration.html#confval-linkcheck_ignore.

[RohitP2005]

Just two more changes, thanks for taking this up, @RohitP2005!

I have fixed those corrections and made a commit.
Thank you @agriyakhetarpal i enjoy working with PyBamm and look forward to contribute more