V2EX = way to explore
V2EX 是一个关于分享和探索的地方
现在注册
已注册用户请  登录
推荐学习书目
Learn Python the Hard Way
Python Sites
PyPI - Python Package Index
http://diveintopython.org/toc/index.html
Pocoo
值得关注的项目
PyPy
Celery
Jinja2
Read the Docs
gevent
pyenv
virtualenv
Stackless Python
Beautiful Soup
结巴中文分词
Green Unicorn
Sentry
Shovel
Pyflakes
pytest
Python 编程
pep8 Checker
Styles
PEP 8
Google Python Style Guide
Code Style from The Hitchhiker's Guide
workwonder
V2EX  ›  Python

Python 's builtin min/max is evil

  •  
  •   workwonder ·
    wonderbeyond · 2017-11-14 19:53:37 +08:00 · 5656 次点击
    这是一个创建于 2326 天前的主题,其中的信息可能已经有所发展或是发生改变。

    最近频繁被 Python 内建的 min 函数坑害,该函数接受两种调用方式: min(a, b, ...)min([a, b, ...]),然后加上其内部容错一点都没有,一不小心就跑挂了,比如参数中有 None,参数为空等 “意外” 情形。鉴于最近多次受灾,我实现了其安全替代: https://gist.github.com/wonderbeyond/fdd874f37d86534f23109f153cb671a2 (自带 doctest )

    65 条回复    2017-11-21 22:26:43 +08:00
    20015jjw
        1
    20015jjw  
       2017-11-14 20:04:54 +08:00 via Android   ❤️ 1
    本来自带函数就是简单为主 强行 evil
    congeec
        2
    congeec  
       2017-11-14 20:09:08 +08:00
    为嘛不用 key=func 参数?
    你说他们 evil,我倒觉得这是强类型的优势
    CSM
        3
    CSM  
       2017-11-14 20:13:59 +08:00 via Android
    有时候快速失败反而易于发现 bug
    zsj950618
        4
    zsj950618  
       2017-11-14 20:14:48 +08:00   ❤️ 3
    >>> min(filter(None, [1,2,4, None]))
    1

    >>> min(filter(None, [1,2,4, '']))
    1

    搞那么复杂干嘛
    introom
        5
    introom  
       2017-11-14 20:16:28 +08:00 via Android
    这个问题我也经常遇到,语言设计的缺陷
    gstqc
        6
    gstqc  
       2017-11-14 20:16:51 +08:00 via iPhone
    楼主强行把 None 去掉……
    如果传入个 bool, str, dict 又怎么办?
    workwonder
        7
    workwonder  
    OP
       2017-11-14 20:20:42 +08:00
    @CSM 我们的业务中,`min(t.start for t in tasks)`,其中 task.start 为 None 是很正常的,所以也不需要快速失败。
    还有如果 tasks 是个空列表,我完全希望其返回 None。
    n2ex2
        8
    n2ex2  
       2017-11-14 20:23:20 +08:00 via Android
    @workwonder 这只是你的需求,别坑其他人。
    workwonder
        9
    workwonder  
    OP
       2017-11-14 20:23:50 +08:00
    @zsj950618 我还考虑了 `min()`, `min(1)`, `min([])`,所以复杂了点。
    workwonder
        10
    workwonder  
    OP
       2017-11-14 20:24:35 +08:00
    @n2ex2 危害在哪儿,欢迎举例。
    n2ex2
        11
    n2ex2  
       2017-11-14 20:32:59 +08:00 via Android   ❤️ 4
    不符合 min/max 逻辑的使用方式就应该报错而不应该过度包装。
    n2ex2
        12
    n2ex2  
       2017-11-14 20:33:57 +08:00 via Android
    你说它没有容错,其实只是你使用方式不严谨而已。
    est
        13
    est  
       2017-11-14 20:40:54 +08:00   ❤️ 1
    这都能够坑,LZ 就是传说中重构火葬场的人吧。

    还是早点放弃动态语言为好

    遇到 js 你岂不是直接疯掉。

    来猜一猜 [1, 2, 3].map(parseInt) 返回多少。
    billgreen1
        14
    billgreen1  
       2017-11-14 20:48:17 +08:00
    危害就是 Python 里 int/float 不可以与 None 比较,你强行让他们可以比较了。再说你可以用 float("nan")代表缺失值,这是标准做法吧?
    dawncold
        15
    dawncold  
       2017-11-14 20:53:43 +08:00
    印象中没遇到过问题,而且用的时候一般很明确非空列表或 generator 中取最大或最小,再就是两个值比最大或最小

    例如:
    ```python
    b2c_stock_base = max(min(v.b2c_stock_base // v.quantity for v in variants), 0)

    shopper_available_count = max(0, privilege_rule.max_count_per_shopper - row.bought_count)
    ```

    你的 task.start 表示什么意思呢?
    workwonder
        16
    workwonder  
    OP
       2017-11-14 20:55:53 +08:00
    @est 让您意外了,确实被坑的狠!而且我 Python 有多年经验。哈哈!

    一开始没做详尽数据数据校验,但是能正确运行很久,其它子模块逻辑微调,意外受灾。但是用我所谓的 safe 版替代,确实不用到处做那么冗杂的判断了,这也从侧面反映了内建的 min/max 不太灵活。

    比如像下面这种逻辑:

    >>> task.start = min(task.start, start)

    原地判断一下参数写起来还是很容易出错的。
    workwonder
        17
    workwonder  
    OP
       2017-11-14 20:58:59 +08:00
    @dawncold task.start 表示一个 task 没有设置开始时间。
    dawncold
        18
    dawncold  
       2017-11-14 20:59:00 +08:00
    @est 看起来是 parseInt 的问题
    workwonder
        19
    workwonder  
    OP
       2017-11-14 21:02:20 +08:00
    > 危害就是 Python 里 int/float 不可以与 None 比较,你强行让他们可以比较了

    @billgreen1 我的命名为 min/max_ignore_none, 其实想表达 None 是未知值,参与比较是没有意义的,所以直接忽略了,不管比大还是比小。
    dawncold
        20
    dawncold  
       2017-11-14 21:05:59 +08:00
    @workwonder 我觉得我会这样写:min(t.start for t in tasks if t.started)
    xujinkai
        21
    xujinkai  
       2017-11-14 21:08:02 +08:00
    早报错,早发现,早治疗
    workwonder
        22
    workwonder  
    OP
       2017-11-14 21:14:02 +08:00
    > 不符合 min/max 逻辑的使用方式就应该报错而不应该过度包装

    @n2ex2 我觉得是否符合逻辑得看场景,比如当我们说 世界首富 的时候考虑到了财富未被统计(p.wealth=None)的人了吗?
    我的某些 task.start=None 在系统里面是正常的;我取 `min(t.start for t in tasks)` 的结果作为 project.start 也是既定规则; tasks 是空列表也是正常的,表示没有任务,那么我让 `min([])` 返回 None 表示没有结果也是符合预期的。
    所以我觉得我没有过度扭曲 min/max 的语义。
    workwonder
        23
    workwonder  
    OP
       2017-11-14 21:16:38 +08:00
    > 我觉得我会这样写:min(t.start for t in tasks if t.started)

    @dawncold 这样并没有覆盖所有 corner case 呀!你应该没注意看问题。你试试:

    >>> min(x for x in [None] if x)
    ValueError: min() arg is an empty sequence
    n2ex2
        24
    n2ex2  
       2017-11-14 21:19:48 +08:00 via Android
    @workwonder 既然是看场景的问题,那就不应该写进基础的函数,你有需求自己改。
    workwonder
        25
    workwonder  
    OP
       2017-11-14 21:28:05 +08:00
    @n2ex2 确实,我实现的 min/max 替代跟我的场景比较契合。
    但我觉得更大范围用于一般场景也没啥问题(这里的语义是取最大最小值,至于数据校验并不是该函数的语义,忽略掉 None 也没关系),你可以看作是 API 风格问题,换个语言的标准库,表现风格都差别很大吧。
    dawncold
        26
    dawncold  
       2017-11-14 21:29:16 +08:00
    @workwonder 我觉得在执行 min(t.start for t in tasks if t.started)之前你应该判断 tasks 是否为空
    workwonder
        27
    workwonder  
    OP
       2017-11-14 21:32:34 +08:00
    @dawncold 完全可以,只是在我的场景里面 min 用的比较多,需要判断的也比较多,最近多次意外挂掉。才搞了个包办的 min/max 实现。
    dawncold
        28
    dawncold  
       2017-11-14 21:36:55 +08:00
    @workwonder 哦,可能模型有些问题? bad smell
    workwonder
        29
    workwonder  
    OP
       2017-11-14 21:41:55 +08:00
    对了,我想额外补充一句,我的场景里面要跟其它子系统对接的,我完全没有对其**数据校验**的需要,也完全没有**尽早报错**的考虑,我能提取到需要的属性则提取之,提出不到就设置为 None,然后我这边继续运行我的。
    SuperMild
        30
    SuperMild  
       2017-11-14 23:06:34 +08:00
    @workwonder 标准库肯定要及早报错的,如果标准库按你那个思路来,才是坑。报错了怎么处理都有自由,不报错让不同需求的人咋办啊。
    xrlin
        31
    xrlin  
       2017-11-14 23:23:18 +08:00 via iPhone
    这只是你的需求,None 本就不应该和数值类型比较,抛出异常才是最常见最保险的做法。
    e9e499d78f
        32
    e9e499d78f  
       2017-11-14 23:25:12 +08:00 via iPhone
    提前 filter 一下就可以了
    lrxiao
        33
    lrxiao  
       2017-11-15 01:45:33 +08:00
    ignore None 还叫 安 全 替 代 ...
    xiaket
        34
    xiaket  
       2017-11-15 06:15:59 +08:00
    min 只负责判断大小,类型处理还是在接收数据的时候搞清楚吧
    billgreen1
        35
    billgreen1  
       2017-11-15 07:43:44 +08:00
    @workwonder None 是空值,不是未知值。缺失值我刚才说了是 float("nan"), 是数值类型。而且我觉得不要相信来自别人的输入是第一要义。
    workwonder
        36
    workwonder  
    OP
       2017-11-15 08:24:26 +08:00 via Android
    @billgreen1 你不觉得 float('inf/nan') 是种很晦暗东西吗,我就当不认识它们,也几乎没见人用过,过于奇怪,比如为啥没有 int 类型的等价物。
    用 None 表示未知(没有设置)我觉得没毛病,而且类型无关。
    billgreen1
        37
    billgreen1  
       2017-11-15 09:11:37 +08:00
    @workwonder 能解决问题就好,不用在这上面纠结。多接触自己不了解的东西,觉得 xx 是很灰暗的东西,就当不认识它们,个人感觉这样不好,有点鸵鸟把头埋进沙子里,就当危险不存在一样。

    不过我发现了一个问题是自带的 min 真不好用。numpy 里面的 min:
    np.min([1,2,np.nan]) ---> nan
    np.min([np.nan, 1,2])---> nan

    但是自带的 min

    min([np.nan, 1, 2]) ---> nan

    min([1,2,np.nan]) ---> 1

    这里面出现了不一致
    workwonder
        38
    workwonder  
    OP
       2017-11-15 09:12:23 +08:00 via Android
    @xiaket 赞同,我就是这个意思。而且我确实不关心里面是否有 None,或者去掉 None 之后是否只剩空列表。
    lrxiao
        39
    lrxiao  
       2017-11-15 09:22:32 +08:00
    @billgreen1 因为 IEEE754 说了因为 NaN 本身没有序 除非-NaN +NaN 比较 不知道 numpy 什么意思
    workwonder
        40
    workwonder  
    OP
       2017-11-15 09:23:25 +08:00 via Android
    @lrxiao 我不仅 ignore none,还忽略了空参数。
    我觉得挺安全。min 能接受任意长度的序列,空序列除外。我在序列为空时返回 None 是自我保护。此时请不要再说数据校验的事儿(很多人在纠结),这里只比大小,逻辑既然走到这,说明这些情况是合理的。
    billgreen1
        41
    billgreen1  
       2017-11-15 09:26:51 +08:00
    @lrxiao numpy 是 Python 的第三方包,可以说是 python 里数值计算类的标准包了。
    zhicheng
        42
    zhicheng  
       2017-11-15 09:38:16 +08:00
    是谁说 None 不能和 int 比较的啊。

    Python 2.7.10 (default, Jul 15 2017, 17:16:57)
    [GCC 4.2.1 Compatible Apple LLVM 9.0.0 (clang-900.0.31)] on darwin
    Type "help", "copyright", "credits" or "license" for more information.
    >>> None > 0
    False
    >>> None < 0
    True
    lrxiao
        43
    lrxiao  
       2017-11-15 09:47:28 +08:00
    @billgreen1 我知道。。我在想为什么 numpy 这么干 大概是早发现 早报错的思路。。
    lrxiao
        44
    lrxiao  
       2017-11-15 09:48:29 +08:00
    @workwonder 那也是你自己的情况。。然后就 evil。。
    workwonder
        45
    workwonder  
    OP
       2017-11-15 10:02:30 +08:00 via Android
    @lrxiao 我承认我的标题有点雷人。发帖前在处理这个问题,把一些 min 的使用场景替换了下,心情比较激动😂

    我的诉求能否普遍适用,大家自己斟酌。

    我觉得把它看做是 API 风格问题也能说通,大家可以看看数据库怎么处理 NULL 的,有自己的一套主见。

    然后,留一个问题大家看怎么解决,就是我为了判断序列是否为空,要提前把迭代器消化成 list,会浪费内存,对于比较长的生成器是不靠谱的。
    workwonder
        46
    workwonder  
    OP
       2017-11-15 10:05:25 +08:00
    @zhicheng 哥们,我刚确认了下 python2 和 python3 不一样

    >>> import sys
    >>> sys.version
    '3.6.2 (default, Jul 17 2017, 23:14:31) \n[GCC 5.4.0 20160609]'
    >>> min([None, 1])
    Traceback (most recent call last):
    File "<stdin>", line 1, in <module>
    TypeError: '<' not supported between instances of 'int' and 'NoneType'
    >>> None > 1
    Traceback (most recent call last):
    File "<stdin>", line 1, in <module>
    TypeError: '>' not supported between instances of 'NoneType' and 'int'
    Chingim
        47
    Chingim  
       2017-11-15 10:09:50 +08:00 via Android
    我觉得 max min 居然是全局函数,这点才 evil 吧。[1,2,3].max() 才符合直觉吧
    xiaket
        48
    xiaket  
       2017-11-15 10:47:29 +08:00
    @Chingim 发现一只异端,按住啦!我去点火! lol

    事实上 sum/all/min/max 这几个全局函数配合 list comprehension 来用效果很好
    ruoyu0088
        49
    ruoyu0088  
       2017-11-15 10:52:45 +08:00
    不仔细看文档吗。min, max 都有 default 参数专门对应参数为空的情况。如果希望空列表返回 None:
    a = [1, 2, None, 4]
    min((v for v in a if v is not None), default=None)
    workwonder
        50
    workwonder  
    OP
       2017-11-15 11:10:35 +08:00
    @ruoyu0088

    心好累!你一定没注意看我的诉求,我完全可以加各种判断,但是场景太多,容易出错。
    你写的表达式并没有覆盖 min_ignore_none 所处理的各种情况,但是已经很长了。

    你试试:

    >>> min(*[], default=1)
    >>> min(*[1], default=None)
    >>> min(1, default=None)

    当然你不会手写这种表达式,但他们在我的场景里面是会出现的
    ruoyu0088
        51
    ruoyu0088  
       2017-11-15 11:29:49 +08:00
    @workwonder 我当然知道你的需求不只是 default,不过我看你的那个安全替代的方案,提前把迭代器消化成 list,就能看出你不知道 default 参数。
    VYSE
        52
    VYSE  
       2017-11-15 11:33:20 +08:00
    可以重构 None 嘛,PY3 可以直接继承 None,PY2 实现等同 None 的 class,然后加__lt__,__gt__,__cmp__呗
    cyrbuzz
        53
    cyrbuzz  
       2017-11-15 11:38:27 +08:00
    可以用 key 和 default 参数实现的简单些。
    ```
    from collections import Iterable

    def _min(*args, **kwargs):
    __min = lambda y: min(y, default=None, key=lambda x: x if x is not None else float('inf'))

    if not args:
    return None

    if not isinstance(args[0], Iterable):
    if len(args) == 1:
    return args[0]
    return __min(args)

    return __min(*args)
    ```
    eloah
        55
    eloah  
       2017-11-15 13:28:33 +08:00
    不爽不要用,只是你自己的需求而已
    起个标题搞个大新闻, naive
    自己写个__cmp__很难吗
    bonfy
        56
    bonfy  
       2017-11-15 14:35:14 +08:00
    不要将滥用说作 evil 啊
    自己的情况自己分析解决就行了,evil 啥了啊...
    repus911
        57
    repus911  
       2017-11-15 18:24:21 +08:00
    Python 's builtin min/max is not evil
    Daetalus
        58
    Daetalus  
       2017-11-15 19:07:35 +08:00   ❤️ 1
    你不应该把数据清理(清洗)步骤和数据处理步骤混在一起。

    对于处理数据的人员来说,进行防御性编程,比如添加“数据校验”并“尽早报错”是常识。你自己武断的去除这两个步骤是不负责任的。追求目前能用就行了,这怎么看都是新手的作风。如果数据传到你这依然有 None,证明前面出了问题,你要么反馈给前面步骤的负责人员。嫌麻烦的话就自己再添加一个数据清理的步骤。

    返回 None 是大忌,最好的做法是报错并给出可靠的错误消息。
    workwonder
        59
    workwonder  
    OP
       2017-11-15 21:05:33 +08:00 via Android
    @Daetalus 我不是说了,我的数据逻辑里面有 None 是正常逻辑,表示未知。
    你可以认为我的做法没有普适性,但你的言论更加武断。
    cy18
        60
    cy18  
       2017-11-15 22:01:11 +08:00
    同意 @n2ex2 #11 的观点,不符合 min/max 逻辑的使用方式就应该报错而不应该过度包装。
    如果自带的 min,max 默认处理 None,这才是 evil 的。可以参考 JS 各种神奇的==结果。
    widewing
        61
    widewing  
       2017-11-15 22:50:55 +08:00 via Android
    确实挺 evil 的,连我 cpu 烧了这种简单的情况都不能自动处理😂
    ryd994
        62
    ryd994  
       2017-11-15 23:14:01 +08:00 via Android
    如果出 none 说明你的代码有问题。语言没有义务给你擦屁股。如果 none 是允许的,你自己把 none filter 掉就好了。
    要是给你把不该隐藏的隐藏了隐藏了,出 unexpected behavior,你是不是有可以骂一次?
    Daetalus
        63
    Daetalus  
       2017-11-15 23:58:41 +08:00
    @workwonder 这不是有没有普适性的问题,而是不应该把数据清理和数据处理的步骤混在一起。在你这个例子中,本质上你还是在真正的 min 和 max 前面加上了一个数据清理的步骤,然后调用 min 或 max。

    数据到你这里还有 None,怎么看都是架构上出了问题。最明显的一个,就是你无法判断一个迭代器是否包含了一堆 None。

    如果是正常的数据,直接迭代一次就能知道序列是否为空。你这种情况还要将迭代器转成列表,以此来判断每个值是否为 None。你自己说说是不是设计上哪里出问题了。

    所以到了调用 min 和 max 的地步,数据中居然还可能为 None、[None]、含有 None 的迭代器,这些都是非常不靠谱的。建议加上一个数据清理的步骤,从逻辑上将两者分开。
    dstwhk
        64
    dstwhk  
       2017-11-21 08:34:02 +08:00
    装逼的人真是恶心,我说的就是你
    装老外,干嘛正文和回复不用英文?
    workwonder
        65
    workwonder  
    OP
       2017-11-21 22:26:43 +08:00 via Android
    @dstwhk 大家的观点都表达清楚了,这贴没啥好讨论得了,也已经沉底,你翻那么深觉得恶心能怪谁。

    我在英文社区交流当然不会用中文,我不介意蹩脚的英文,我会尽力清晰表达自己的意思,欢迎围观: https://github.com/wonderbeyond
    关于   ·   帮助文档   ·   博客   ·   API   ·   FAQ   ·   我们的愿景   ·   实用小工具   ·   1116 人在线   最高记录 6543   ·     Select Language
    创意工作者们的社区
    World is powered by solitude
    VERSION: 3.9.8.5 · 31ms · UTC 18:49 · PVG 02:49 · LAX 11:49 · JFK 14:49
    Developed with CodeLauncher
    ♥ Do have faith in what you're doing.