madwind / flexget_qbittorrent_mod

flexget qbittorrent删种、辅种 自动签到 插件
MIT License
552 stars 117 forks source link

fix mypy error: Cannot override writable attribute "USER_CLASSES" with a final one #90

Closed vivodi closed 2 years ago

vivodi commented 2 years ago

被标记为 Final 的属性不能覆写基类中的非只读属性。本PR将基类中被子类 Final 属性覆写的属性设为只读。

madwind commented 2 years ago

什么叫不能覆写基类中的非只读属性?我测试是没有问题的,改成方法太难看,且不符合方法命名规范

vivodi commented 2 years ago

你要用 Mypy(Python官方的类型检查器) 测试,PyCharm内置的类型检查不完善。

class Base:
    a = 1

class Derived(Base):
    a: Final = 2

mypy会这样报错:

test.py:9: error: Cannot override writable attribute "a" with a final one

Base 中,a是可写入的,不能被子类的 Final 属性覆写,改为用 @property 后就是只读的,可以被子类的 Final 属性覆写

可以参考mypy官方文档

Note however that a final attribute can override a read-only property:

class Base:
    @property
    def ID(self) -> int: ...

class Derived(Base):
    ID: Final = 1  # OK
vivodi commented 2 years ago

上面mypy官方文档中 ID 方法也是大写

madwind commented 2 years ago

我还是觉得不好看,子类的定义和父类不一致,这种行为看起来就很怪,实在不行就忽略这个错误

madwind commented 2 years ago

或者不定义成Fianl也比搞这么一段怪异的代码好

vivodi commented 2 years ago

我感觉忽略这个 error 不好,因为

  1. 这是 error 不是 warning
  2. 这个error 是mypy的 非strict模式 下报的,mypy 的 strict模式 甚至还有更多约束

我选择将所有的 Final 全部去掉。

你看是添加 ClassVar 好还是什么也不添加好?

madwind commented 2 years ago

这个error并没有什么影响,我不知道python还有什么其它定义,但在这个应用里它就是一个常量,不应被修改。且父类是不可实例化的,所以我不觉得在这种情景下它会有什么影响。 没有警告,或者报错的话就不加了,如果有,那就加吧

vivodi commented 2 years ago

我还是倾向于改为 @property 的函数,因为mypy的官方文档中出现了这种写法,并且mypy的文档中认为这是合理的。而且将属性改为用@property 的函数访问是Python中将属性改为只读的常用方法之一,参见https://stackoverflow.com/a/45068730/12340650

如果不改为 @property 的函数访问,那么下面的代码也是合理的,MainClass 可以修改 VisitHR 中定义 SUCCEED_REGEX

class VisitHR(NexusPHP, ABC):
    SUCCEED_REGEX = '[欢歡]迎回[来來家]'

class MainClass(VisitHR)  # ptsbao.py
    def f(self):
        self.SUCCEED_REGEX = '...' #SUCCEED_REGEX应该是常量,不应被再次赋值

而这显然是不合理的行为。而将 SUCCEED_REGEX 改为 @property 的函数访问就不会有这个问题,因为它成为了只读属性。

vivodi commented 2 years ago

本来上述代码可以用 Final 标记来达到避免被重新赋值的目的,但是Final属性不能被子类覆写,所以将父类中的常量属性改为 @property 的函数,避免常量被重新赋值

vivodi commented 2 years ago

如果你还是觉得这样不好,我也更倾向于将所有的 Final 都去掉而不是忽略error。

还有一个办法,就是移除父类中被子类Final属性覆写的属性,或者在父类中直接实现该属性,避免子类覆写。即,将VisitHR中的SUCCEED_REGEX放到ptsbao中,因为它的子类只有ptsbao使用了该SUCCEED_REGEX。将PrivateTorrent中的USER_CLASSES设为抽象属性,要求所有子类都实现USER_CLASSES。但是这反而是为了这个问题而去对程序其他地方进行修改,也说不上是一个好办法。

vivodi commented 2 years ago

为了说明父类中被子类Final属性覆写的属性应该用@property 函数设为只读,我再举个例子

class VisitHR(NexusPHP, ABC):
    SUCCEED_REGEX = '[欢歡]迎回[来來家]'
    def f(self):
        self.SUCCEED_REGEX = '欢迎回来'

class PtsbaoClass(VisitHR):  # ptsbao.py
    pass

class CcfbitsClass(VisitHR):  # ccfbits.py
    SUCCEED_REGEX: Final = '欢迎回到CCFBits'

ptsbao = PtsbaoClass()
ptsbao.f() #SUCCEED_REGEX为 普通 属性。不会报错
ccfbits = CcfbitsClass()
ccfbits.f() #SUCCEED_REGEX为 Final 属性,不应被重新赋值

由于多态性,如果不将父类中的SUCCEED_REGEX设为只读,PtsbaoClass的实例可以修改SUCCEED_REGEX,而CcfbitsClass的实例不能。

这会导致两个子类调用父类中同一方法的行为不一致。这也是父类中被子类 Final 属性覆写的属性应该设为只读属性的深层次原因