Travis 3.6 Slow Build Test Failures - BS4 Issue

[alimcmaster1]

test_thousands_macau_index_col in test_html fails using beautifulsoup4 version 4.8.1 and passes using version 4.7.1.

Example failures:
https://travis-ci.org/pandas-dev/pandas/jobs/611742966

As pointed out by @jreback in this PR - #29603

Example failing build pre my changes in #29513
https://travis-ci.org/pandas-dev/pandas/jobs/611347428

Attempting to debug issue and will report to bs4. Should we pin in the meantime to version <= 4.7.1?

[simonjayhawkins]

duplicate of #28125

[nschloe]

Bumped into this as well, downgrading to beautifulsoup4 4.7.1 fixed it for me.

[jreback]

would really really like to fix / xfail this test. ! cc @alimcmaster1

[alimcmaster1]

take

[leonardr]

This was brought to my attention through Beautiful Soup bug 1857765. I've fixed the problem on the Beautiful Soup side in revision 533. However, in normal usage Tag.attrs should never be None. Looking at the test that's failing I think there's a problem in pandas code, though the fix isn't obvious to me.

This code iterates over a list of tables and calls decompose() on anything inside that table with display:none. I think the problem happens when you have nested tables, and processing table N involves calling decompose() on table N+1 (because table N+1 is inside table N and has display:none).

Once you decompose() a Tag its __dict__ is wiped. That means calling Tag.attrs will invoke Tag.__getattr__, which will try to find a child tag called "attrs". That will return None, of course. So that's how Tag.attrs ends up None.

In general, once you call decompose() on a Tag object its behavior becomes undefined and you're not supposed to use it for anything. Here it looks like you're not intentionally using a decomposed Tag, but you're iterating over a list and some Tags may have been decomposed by the time you get to them.

Since you're only adding table to a list if a find call returns a value other than None, and find on a decomposed Tag will always return None, I don't think this problem has any real-world effects. It would be better to avoid dealing with a decomposed Tag altogether, but right now there's no good way to tell whether a Tag has been decomposed.

I could change the behavior of Tag so that attempts to use a decomposed Tag raise an exception, but that would just make the problem more obvious -- you'd get a different exception inside Tag.__unicode__. So maybe I should set a flag on a Tag that's been decomposed; that way you can check for it. Let me know what you think.

[alimcmaster1]

Thanks very much @leonardr for taking a look into this. I agree with you we should look into this on the pandas side. Ill take a look and reply back on here

I can confirm your change in revision 553 fix our test on master. https://bazaar.launchpad.net/~leonardr/beautifuI lsoup/bs4/revision/553

@jreback I have xfailed our tests for now and then will remove when beautifulsoup changes are released: see PR #30544

@leonardr would be much appreciated if you ping us on here when you aim to next release

[leonardr]

I can do a release once we figure out how to deal with the underlying situation -- a Tag might have been decomposed and there's no way to tell.

[alimcmaster1]

I can do a release once we figure out how to deal with the underlying situation -- a Tag might have been decomposed and there's no way to tell.

Agree thanks for the detailed explanation above. Defer to you really on how you want to handle this and we can update the pandas code accordingly.

Here it looks like you're not intentionally using a decomposed Tag, but you're iterating over a list and some Tags may have been decomposed by the time you get to them.

Could bs4 provide functionality to safely iterate over a list? Might be easier than callers handling a flag?

Otherwise the flag approach to identify if .decompose() has been called on the Tag sounds good or calling .decompose() a second time a no-op?

@leonardr

[leonardr]

Iterating over a list isn't dangerous per se; the problem happens when you try to take an action on a decomposed PageElement. That can happen any time you decompose a tag you've assigned to a variable.

While working out a solution I noticed that Tag.name will be None for a decomposed tag. If you want to get everything working again without waiting for a new Beautiful Soup release or updating dependencies, I think that's an easy and intuitive way to check whether a tag has been decomposed, especially in the context where you specifically looked for table tags.

I may still need to change Beautiful Soup, since there's no way to tell whether a NagivableString has been decomposed, but since that's going to change the API I'd like to wait a bit.

[TomAugspurger]

@alimcmaster1 do you know what all remains to be done on pandas' side here? Pushing off the 1.0 milestone, as I don't think there's anything critical, but I haven't been following closely.

[alimcmaster1]

Agree nothing critical @TomAugspurger i can follow up shortly with the fix @leonardr suggested and hence remove the xfail added in #30544

[leonardr]

FYI I just released Beautiful Soup 4.9.0 with the fix discussed in this issue.

[alimcmaster1]

Awesome thanks vm!! @leonardr