Remove inference tip cache

[kriek]

ChangeLog

Fixed type inconsistently inferred for base methods returning type(self).

Description

Remove inference tip cache. It misses the context to properly evaluate a base class method returned type when the returned type depends on the subclass instance calling the method.

Type of Changes

	Type
✓	🐛 Bug fix

Related Issue

Closes #1828

[coveralls]

Pull Request Test Coverage Report for Build 3245692888

• 6 of 6 (100.0%) changed or added relevant lines in 2 files are covered.
• No unchanged relevant lines lost coverage.
• Overall coverage decreased (-0.009%) to 92.288%

---

Totals
Change from base Build 3245635291:	-0.009%
Covered Lines:	9848
Relevant Lines:	10671

---

💛 - Coveralls

[cdce8p]

I'm not sure this is the right call. It looks like we don't fully understand the consequences of removing the cache yet.
While testing with Home Assistant, I'm seeing a noticeable performance impact (from 7min previously to 8-10min) and a new false-positive.

[DanielNoord]

I was going to ask you for a test on home-assistant. Thanks for already doing so.

That does indeed seem like this serves a purpose!

[kriek]

I'm not sure this is the right call. It looks like we don't fully understand the consequences of removing the cache yet. While testing with Home Assistant, I'm seeing a noticeable performance impact (from 7min previously to 8-10min) and a new false-positive.

Would you mind sharing the false-positive location? I'd like to dig that a bit.
I'm also having a look at the performance impact but my Windows machine seems not very suited for the home-assistant pylint evaluation and for now, I'm fighting to get the pip requirements installed.

[Pierre-Sassoulas]

Regarding the performance impact, it's expected that removing the cache would lower performance. Slightly lower perfs are acceptable if we fix a bug that happen often, but +10% is not slight. We'd need to replace the buggy cache by a working cache. What was the false positive in home-assistant @cdce8p ?

[kriek]

I'm not able to see the false positive on my side. On my slow machine, the home-assistant penalty is +15% (from 28 to 32 min ; with --jobs=1). Indeed not slight.

[cdce8p]

What was the false positive in home-assistant @cdce8p ?

Haven't had time to double check. Hopefully tomorrow.

[cdce8p]

Would you mind sharing the false-positive location? I'd like to dig that a bit.

homeassistant/components/hive/__init__.py:92:8: E0705: Exception cause set to something which is not an exception, nor None (bad-exception-cause)

https://github.com/home-assistant/core/blob/beeee8b60eaa2415add69cd0b385dec56182a407/homeassistant/components/hive/__init__.py#L90-L92

pylint -j 1 homeassistant/components/hive/__init__.py 

This seems to be enough to reproduce it.

I'm also having a look at the performance impact but my Windows machine seems not very suited for the home-assistant pylint evaluation and for now, I'm fighting to get the pip requirements installed.

I've given up on running it locally a long time ago 😅 My laptop just isn't powerful enough.
My performance numbers where from Github action runs which is probably a bit inaccurate.

I'm not able to see the false positive on my side. On my slow machine, the home-assistant penalty is +15% (from 28 to 32 min ; with --jobs=1). Indeed not slight.

+15% roughly matches what I've seen. IMO that's quite a lot so I'm really not sure it's worth it to remove the whole cache.
I haven't followed the discussion around the issue. Ideally we would find why caching doesn't work properly and fix it instead of removing it.

[jacobtylerwalls]

Thanks for the investigation @kriek! Your analysis of the problem is correct. I added your regression test and credited you in #2158, which just fixes the cache key, as suggested by @cdce8p in his last message. Closing this PR as superseded.

[kriek]

Thanks for moving forward by tackling the cache key issue @jacobtylerwalls!