Fix Lychee link failures from stack overflow

[kratman]

Description

Removes stack overflow links due to lychee failures

Type of change

Fixes Lychee link checker

• [x] Bug fix (non-breaking change which fixes an issue)

Key checklist:

• [x] 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, in just two lines of code)
• [x] All tests pass: $ python run-tests.py --all (or $ nox -s tests)
• [x] The documentation builds: $ python run-tests.py --doctest (or $ nox -s doctests)

You can run integration tests, unit tests, and doctests together at once, using $ python run-tests.py --quick (or $ nox -s quick).

Further checks:

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

[codecov[bot]]

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 99.59%. Comparing base (1a5c4e4) to head (99376b5). Report is 1 commits behind head on develop.

Additional details and impacted files

@@           Coverage Diff            @@
##           develop    #3989   +/-   ##
========================================
  Coverage    99.59%   99.59%           
========================================
  Files          258      258           
  Lines        21310    21310           
========================================
  Hits         21223    21223           
  Misses          87       87           

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

[agriyakhetarpal]

I would be slightly against this change. Usually link checkers are used to prevent link rot and make sure that dead links are not scattered across codebases. However, StackOverflow permalinks will never go extinct as long as StackOverflow never goes down, so, could we ignore the website altogether?

[kratman]

@agriyakhetarpal, @Saransh-cpp In general I don't think adding links to code snippets is all that useful. I don't think many people would just copy the link to look at it. They would then have to read a whole blog post or forum post just to find the key pieces of information. If there is something that important in the link, then it should be explained in plain text comments. I strongly believe comments should be minimized, but their main purpose is to explain extremely complex parts of the code. If somebody changes a code block commented by a link, then the person updating that code block has to read the link to determine if they changed what was commented in there. This can easily lead to code rot in the comments.

While we can mostly assume that some sites will never change their domains or delete pages, the whole point of link checking is to confirm that the assumption is correct. If we suppress any site which is not perfect for the link checker, then what is the point of the link checker? There are some things like the paths to executables which the link checker will never be able to handle, but the point is to check if links work. If somebody adds a new stack overflow link in a comment do you think all reviewers are going to copy the link to check it or do you think they will see the link checker pass and assume that the link is correct?

Additionally the link checker has repeatedly marked PRs as failing their CI checks, which hides real problems. I am going to go back through and update the comments in the parts @tinosulzer flagged, but I think in general reducing the ways in which things can fail or break is an improvement.

[kratman]

Real link failure that was hidden by the StackOverflow ones: https://github.com/keras-team/keras/blob/master/keras/callbacks/callback.py | Failed: Network error: Not Found

[agriyakhetarpal]

@kratman and I discussed this outside of GitHub recently and I had pitched for the creation of an automated issue in a workflow that runs fortnightly (or monthly would be better) instead of on PRs (which I would be happy to look at the failures for and be assigned to the issue, and also modify the workflows to do this). This is how 2i2c is doing it, and they do so monthly: https://github.com/2i2c-org/2i2c-org.github.io/issues/229. While it doesn't hide the problem of whether StackOverflow links should be included and they can get very outdated, this could serve as a reasonable alternative to let CI checks display as passing while having flaky links and therefore mitigate ❌s showing up before a ✅ does

I think the point of a link checker is more about verifying internal links (such as those in the docs, which we have #3387 open for) or for those links that are absolutely needed, so reducing the current ~540 links should also be great to do sometime or the other. This comment still doesn't resolve how to navigate the "providing-attribution-vs-removing-links-to-it" but I wish to advocate for reducing non-useful links but also keeping useful ones.

[kratman]

I had pitched for the creation of an automated issue in a workflow that runs fortnightly (or monthly would be better) instead of on PRs

I think it is better to have the link checker run on individual PRs if we are going to do it. The purpose should be to check if a new change has added a bad link

[kratman]

@valentinsulzer, @Saransh-cpp Can you re-review I would like to get this link checker fixed

[valentinsulzer]

Thanks! @Saransh-cpp I think that addresses your main concern as well - just waiting for your approval now

[kratman]

I dismissed @Saransh-cpp's review since he said previously:

Please feel free to merge if everyone else is happy with it.