Python 's builtin min/max is evil

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

min

Python

函数

参数

65 条回复 • 2017-11-21 22:26:43 +08:00

20015jjw

2017-11-14 20:04:54 +08:00 via Android

本来自带函数就是简单为主强行 evil

congeec

2017-11-14 20:09:08 +08:00

为嘛不用 key=func 参数？
你说他们 evil，我倒觉得这是强类型的优势

CSM

2017-11-14 20:13:59 +08:00 via Android

有时候快速失败反而易于发现 bug

zsj950618

2017-11-14 20:14:48 +08:00

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

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

搞那么复杂干嘛

introom

2017-11-14 20:16:28 +08:00 via Android

这个问题我也经常遇到，语言设计的缺陷

0ZXYDDu796nVCFxq

2017-11-14 20:16:51 +08:00 via iPhone

楼主强行把 None 去掉……
如果传入个 bool, str, dict 又怎么办？

workwonder

2017-11-14 20:20:42 +08:00

@CSM 我们的业务中，`min(t.start for t in tasks)`，其中 task.start 为 None 是很正常的，所以也不需要快速失败。
还有如果 tasks 是个空列表，我完全希望其返回 None。

n2ex2

2017-11-14 20:23:20 +08:00 via Android

@workwonder 这只是你的需求，别坑其他人。

workwonder

2017-11-14 20:23:50 +08:00

@zsj950618 我还考虑了 `min()`, `min(1)`, `min([])`，所以复杂了点。

workwonder

2017-11-14 20:24:35 +08:00

@n2ex2 危害在哪儿，欢迎举例。

n2ex2

2017-11-14 20:32:59 +08:00 via Android

不符合 min/max 逻辑的使用方式就应该报错而不应该过度包装。

n2ex2

2017-11-14 20:33:57 +08:00 via Android

你说它没有容错，其实只是你使用方式不严谨而已。

est

2017-11-14 20:40:54 +08:00

这都能够坑，LZ 就是传说中重构火葬场的人吧。

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

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

来猜一猜 [1, 2, 3].map(parseInt) 返回多少。

billgreen1

2017-11-14 20:48:17 +08:00

危害就是 Python 里 int/float 不可以与 None 比较，你强行让他们可以比较了。再说你可以用 float("nan")代表缺失值，这是标准做法吧？

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

2017-11-14 20:55:53 +08:00

@est 让您意外了，确实被坑的狠！而且我 Python 有多年经验。哈哈！

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

比如像下面这种逻辑：

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

原地判断一下参数写起来还是很容易出错的。

workwonder

2017-11-14 20:58:59 +08:00

@dawncold task.start 表示一个 task 没有设置开始时间。

dawncold

2017-11-14 20:59:00 +08:00

@est 看起来是 parseInt 的问题

workwonder

2017-11-14 21:02:20 +08:00

> 危害就是 Python 里 int/float 不可以与 None 比较，你强行让他们可以比较了

@billgreen1 我的命名为 min/max_ignore_none, 其实想表达 None 是未知值，参与比较是没有意义的，所以直接忽略了，不管比大还是比小。

dawncold

2017-11-14 21:05:59 +08:00

@workwonder 我觉得我会这样写：min(t.start for t in tasks if t.started)

xujinkai

2017-11-14 21:08:02 +08:00

早报错，早发现，早治疗

workwonder

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

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

2017-11-14 21:19:48 +08:00 via Android

@workwonder 既然是看场景的问题，那就不应该写进基础的函数，你有需求自己改。

workwonder

2017-11-14 21:28:05 +08:00

@n2ex2 确实，我实现的 min/max 替代跟我的场景比较契合。
但我觉得更大范围用于一般场景也没啥问题（这里的语义是取最大最小值，至于数据校验并不是该函数的语义，忽略掉 None 也没关系），你可以看作是 API 风格问题，换个语言的标准库，表现风格都差别很大吧。

dawncold

2017-11-14 21:29:16 +08:00

@workwonder 我觉得在执行 min(t.start for t in tasks if t.started)之前你应该判断 tasks 是否为空

workwonder

2017-11-14 21:32:34 +08:00

@dawncold 完全可以，只是在我的场景里面 min 用的比较多，需要判断的也比较多，最近多次意外挂掉。才搞了个包办的 min/max 实现。

dawncold

2017-11-14 21:36:55 +08:00

@workwonder 哦，可能模型有些问题？ bad smell

workwonder

2017-11-14 21:41:55 +08:00

对了，我想额外补充一句，我的场景里面要跟其它子系统对接的，我完全没有对其**数据校验**的需要，也完全没有**尽早报错**的考虑，我能提取到需要的属性则提取之，提出不到就设置为 None，然后我这边继续运行我的。

SuperMild

2017-11-14 23:06:34 +08:00

@workwonder 标准库肯定要及早报错的，如果标准库按你那个思路来，才是坑。报错了怎么处理都有自由，不报错让不同需求的人咋办啊。

xrlin

2017-11-14 23:23:18 +08:00 via iPhone

这只是你的需求，None 本就不应该和数值类型比较，抛出异常才是最常见最保险的做法。

e9e499d78f

2017-11-14 23:25:12 +08:00 via iPhone

提前 filter 一下就可以了

lrxiao

2017-11-15 01:45:33 +08:00

ignore None 还叫安全替代 ...

xiaket

2017-11-15 06:15:59 +08:00

min 只负责判断大小，类型处理还是在接收数据的时候搞清楚吧

billgreen1

2017-11-15 07:43:44 +08:00

@workwonder None 是空值，不是未知值。缺失值我刚才说了是 float("nan"), 是数值类型。而且我觉得不要相信来自别人的输入是第一要义。

workwonder

2017-11-15 08:24:26 +08:00 via Android

@billgreen1 你不觉得 float('inf/nan') 是种很晦暗东西吗，我就当不认识它们，也几乎没见人用过，过于奇怪，比如为啥没有 int 类型的等价物。
用 None 表示未知(没有设置)我觉得没毛病，而且类型无关。

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

2017-11-15 09:12:23 +08:00 via Android

@xiaket 赞同，我就是这个意思。而且我确实不关心里面是否有 None，或者去掉 None 之后是否只剩空列表。

lrxiao

2017-11-15 09:22:32 +08:00

@billgreen1 因为 IEEE754 说了因为 NaN 本身没有序除非-NaN +NaN 比较不知道 numpy 什么意思

workwonder

2017-11-15 09:23:25 +08:00 via Android

@lrxiao 我不仅 ignore none，还忽略了空参数。
我觉得挺安全。min 能接受任意长度的序列，空序列除外。我在序列为空时返回 None 是自我保护。此时请不要再说数据校验的事儿(很多人在纠结)，这里只比大小，逻辑既然走到这，说明这些情况是合理的。

billgreen1

2017-11-15 09:26:51 +08:00

@lrxiao numpy 是 Python 的第三方包，可以说是 python 里数值计算类的标准包了。

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

2017-11-15 09:47:28 +08:00

@billgreen1 我知道。。我在想为什么 numpy 这么干大概是早发现早报错的思路。。

lrxiao

2017-11-15 09:48:29 +08:00

@workwonder 那也是你自己的情况。。然后就 evil。。

workwonder

2017-11-15 10:02:30 +08:00 via Android

@lrxiao 我承认我的标题有点雷人。发帖前在处理这个问题，把一些 min 的使用场景替换了下，心情比较激动😂

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

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

然后，留一个问题大家看怎么解决，就是我为了判断序列是否为空，要提前把迭代器消化成 list，会浪费内存，对于比较长的生成器是不靠谱的。

workwonder

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

2017-11-15 10:09:50 +08:00 via Android

我觉得 max min 居然是全局函数，这点才 evil 吧。[1,2,3].max() 才符合直觉吧

xiaket

2017-11-15 10:47:29 +08:00

@Chingim 发现一只异端，按住啦！我去点火！ lol

事实上 sum/all/min/max 这几个全局函数配合 list comprehension 来用效果很好

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

2017-11-15 11:10:35 +08:00

@ruoyu0088

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

你试试：

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

当然你不会手写这种表达式，但他们在我的场景里面是会出现的

ruoyu0088

2017-11-15 11:29:49 +08:00

@workwonder 我当然知道你的需求不只是 default，不过我看你的那个安全替代的方案，提前把迭代器消化成 list，就能看出你不知道 default 参数。

VYSE

2017-11-15 11:33:20 +08:00

可以重构 None 嘛,PY3 可以直接继承 None,PY2 实现等同 None 的 class,然后加__lt__,__gt__,__cmp__呗

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)
```

cyrbuzz

2017-11-15 11:40:33 +08:00

https://gist.github.com/HuberTRoy/df18d5e5c34f7d965ea1426565b1045b

eloah

2017-11-15 13:28:33 +08:00

不爽不要用,只是你自己的需求而已
起个标题搞个大新闻, naive
自己写个__cmp__很难吗

bonfy

2017-11-15 14:35:14 +08:00

不要将滥用说作 evil 啊
自己的情况自己分析解决就行了，evil 啥了啊...

repus911

2017-11-15 18:24:21 +08:00

Python 's builtin min/max is not evil

Daetalus

2017-11-15 19:07:35 +08:00

你不应该把数据清理（清洗）步骤和数据处理步骤混在一起。

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

返回 None 是大忌，最好的做法是报错并给出可靠的错误消息。

workwonder

2017-11-15 21:05:33 +08:00 via Android

@Daetalus 我不是说了，我的数据逻辑里面有 None 是正常逻辑，表示未知。
你可以认为我的做法没有普适性，但你的言论更加武断。

cy18

2017-11-15 22:01:11 +08:00

同意 @n2ex2 #11 的观点，不符合 min/max 逻辑的使用方式就应该报错而不应该过度包装。
如果自带的 min，max 默认处理 None，这才是 evil 的。可以参考 JS 各种神奇的==结果。

widewing

2017-11-15 22:50:55 +08:00 via Android

确实挺 evil 的，连我 cpu 烧了这种简单的情况都不能自动处理😂

ryd994

2017-11-15 23:14:01 +08:00 via Android

如果出 none 说明你的代码有问题。语言没有义务给你擦屁股。如果 none 是允许的，你自己把 none filter 掉就好了。
要是给你把不该隐藏的隐藏了隐藏了，出 unexpected behavior，你是不是有可以骂一次？

Daetalus

2017-11-15 23:58:41 +08:00

@workwonder 这不是有没有普适性的问题，而是不应该把数据清理和数据处理的步骤混在一起。在你这个例子中，本质上你还是在真正的 min 和 max 前面加上了一个数据清理的步骤，然后调用 min 或 max。

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

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

所以到了调用 min 和 max 的地步，数据中居然还可能为 None、[None]、含有 None 的迭代器，这些都是非常不靠谱的。建议加上一个数据清理的步骤，从逻辑上将两者分开。

dstwhk

2017-11-21 08:34:02 +08:00

装逼的人真是恶心，我说的就是你
装老外，干嘛正文和回复不用英文？

workwonder

2017-11-21 22:26:43 +08:00 via Android

@dstwhk 大家的观点都表达清楚了，这贴没啥好讨论得了，也已经沉底，你翻那么深觉得恶心能怪谁。

我在英文社区交流当然不会用中文，我不介意蹩脚的英文，我会尽力清晰表达自己的意思，欢迎围观： https://github.com/wonderbeyond