ENH: pd.read_html argument to extract hrefs along with text from cells
- [x] closes #13141
- [x] [Tests added and passed](https://pandas.pydata.org/pandas-docs/dev/development/contributing_codebase.html#writing-tests) if fixing a bug or adding a new feature
- [x] All [code checks passed](https://pandas.pydata.org/pandas-docs/dev/development/contributing_codebase.html#pre-commit).
- [x] Added an entry in the latest `doc/source/whatsnew/vX.X.X.rst` file if fixing a bug or adding a new feature.

> Could you please fix the pre-commit checks?

Ah I missed that error - sorry about that. I've fixed it now. This looks fine to me, albeit there is some agreement on the name of the new kwarg. Not convinced by `extract_hrefs`, but I can't right now suggest anything better. > This looks fine to me, albeit there is some agreement on the name of the new kwarg. Not convinced by `extract_hrefs`, but I can't right now suggest anything better.

I felt it was more explicit than `extract_links` - but perhaps that is a better name? @attack68 Could you help with this request from jreback? I'm not sure how to go about testing it.
> need a test that validatees the extract_href arg is among the required values or raises. it will look something like this:

```python
def test_extract_links_bad():
    input = ... # some valid input
    msg = "`extract_links` must be one of {None, "header", "footer", "body", "all"}, got"
    with pytest.raises(ValueError, match=msg):
        df = read_html(input, extract_links="bad_input")
``` > it will look something like this:
> 
> ```python
> def test_extract_links_bad():
>     input = ... # some valid input
>     msg = "`extract_links` must be one of {None, "header", "footer", "body", "all"}, got"
>     with pytest.raises(ValueError, match=msg):
>         df = read_html(input, extract_links="bad_input")
> ```

Yes, but this does not raise an error as there is no runtime type checking. Should I add a `raise ValueErrror` in the `read_html` function? > > it will look something like this:
> > ```python
> > def test_extract_links_bad():
> >     input = ... # some valid input
> >     msg = "`extract_links` must be one of {None, "header", "footer", "body", "all"}, got"
> >     with pytest.raises(ValueError, match=msg):
> >         df = read_html(input, extract_links="bad_input")
> > ```
> 
> Yes, but this does not raise an error as there is no runtime type checking. Should I add a `raise ValueErrror` in the `read_html` function?

I thought there might be a smarter way than just raising if the value isn't in a hard-coded list, but I guess not so unfortunately. I tried using `typing.get_args` / `read_html.__annotations__` but the `extract_links` param was a string rather than a `typing.Literal` for some reason, so I couldn't get a tuple of the allowed args to check against. > It would be good if you can put it all into one test and use. `@pytest.mark.parametrize`

I feel like this would be a bit difficult since the expected DFs won't fit on one line, and it'll all become very long and messy. > > > it will look something like this:
> > > ```python
> > > def test_extract_links_bad():
> > >     input = ... # some valid input
> > >     msg = "`extract_links` must be one of {None, "header", "footer", "body", "all"}, got"
> > >     with pytest.raises(ValueError, match=msg):
> > >         df = read_html(input, extract_links="bad_input")
> > > ```
> > 
> > 
> > Yes, but this does not raise an error as there is no runtime type checking. Should I add a `raise ValueErrror` in the `read_html` function?
> 
> I thought there might be a smarter way than just raising if the value isn't in a hard-coded list, but I guess not so unfortunately. I tried using `typing.get_args` / `read_html.__annotations__` but the `extract_links` param was a string rather than a `typing.Literal` for some reason, so I couldn't get a tuple of the allowed args to check against.

looks like you fixed it with arg validation. > > It would be good if you can put it all into one test and use. `@pytest.mark.parametrize`
> 
> I feel like this would be a bit difficult since the expected DFs won't fit on one line, and it'll all become very long and messy.

you could create a fixture template and populate it with necessary sections and then test.

e.g.

```python
@pytest.fixture()
def base_html():
    return """
        <table>
           <thead>
               <tr> <th>{0}</th></tr>
          </thead><tbody>
               <tr> <th>{1}</th></tr>
          </body><tfoot>
               <tr> <th>{2}</th></tr>
          </tfoot></table>"""


def test_something(base_html):
    html = base_html.format("header cell", "body cell", "footer cell")
    ...test parsing
``` > you could create a fixture template and populate it with necessary sections and then test.

Is it OK now? @attack68 Also, any ideas regarding @jreback's request?
> can you type the args and returns of all of the added code > I do not know why np.nan is needed, rather than None.

Perhaps this is occurring at the engine level (None -> NaN for columns - or specifically MultiIndex columns)?

> There is not test if cell contains multiple links, e.g. "L1 <a href=".." /a> L2 <a href=".." /a>" or even if a cell contains Text followed by a link. WHat is the return objective in these cases?

Ah, I definitely missed these cases. I've added them now. For the former (surrounding text), all tag text is extracted. For the latter, only the first link is captured. @attack68 As I suspected above, both of these issues are artefacts of the parser converting tuple indexes into a MultiIndex:
https://github.com/pandas-dev/pandas/blob/40fbd5903e84b06a7cfcbbd26e28636e5d1d486d/pandas/io/parsers/base_parser.py#L409

https://github.com/pandas-dev/pandas/blob/40fbd5903e84b06a7cfcbbd26e28636e5d1d486d/pandas/io/parsers/base_parser.py#L1330

I pushed a commit (PoC of course, not the final solution) using frozenset to bypass the `isinstance`, and it worked OK, except that now indexing isn't possible since the frozensets are not casted to tuples like I had initially assumed.

Obviously I don't understand most of the moving parts, but it does feel that a bit hasty to convert all tuple indices into a MultiIndex. Thoughts on how to work around this? @attack68 Any suggestions on how I should proceed? @attack68 I've managed to get it in the exact form I wanted (Index instead of MultiIndex, and np.nan replaced with None) but I'm not so sure about the former change anymore. Is it better to maintain consistency in the behaviour of read_html (returning MultiIndexes instead of Index for multi-value headers) or is it OK to do what I have done and make an exception for a case which isn't similar to the others? If you could advise on this, I would appreciate that very much. Hello @abmyii! Thanks for updating this PR. We checked the lines you've touched for [PEP 8](https://www.python.org/dev/peps/pep-0008) issues, and found:





There are currently no PEP 8 issues detected in this Pull Request. Cheers! :beers: 

##### Comment last updated at 2022-08-16 01:47:00 UTC > lgtm, if rebased and passing tests

Rebased and tests are passing. I've kept the extract_links index structure custom (returns a flat tuple index rather than a MultiIndex) for now, but I'm open to reverting that.

I'm not sure what is causing the Docstring action to fail - the line is totally different to what is shown in the error. this is failing checks.eg pandas/io/html.py line 1024. you cab review the logs for explanations. > this is failing checks.eg pandas/io/html.py line 1024. you cab review the logs for explanations.

https://github.com/abmyii/pandas/blob/read_html-extract-hrefs/pandas/io/html.py#L1024 is a blank line and 1025 is not a docstring?

Is https://github.com/abmyii/pandas/blob/read_html-extract-hrefs/pandas/io/html.py#L1141 the issue - and if so, should it be `.. versionadded :: 1.5.0`? Thanks for sticking with this @abmyii!  > Thanks for sticking with this @abmyii!

Awesome, thank you to all the reviewers, and you and @attack68 especially for helping me throughout this process!