Комментарии 27
admin = get_admin() or user or get_user()Конечно на вкус и цвет, но вот эта цепочка or как то не очень выглядит, трудно читать. Лучше через простые if elif else
admin = user if user else get_user()Лучше объекты явно проверять объекты на None, в будущем могут появится баги из за этого
return sum(factors) >= 2Здесь лучше тоже явно кажется, обычным фориком пробежался и все, понятнее будет программисту который читает твой код
for i in range(max_pages + 1):
rsp = client.get_all_users(page=page, limit=limit)
if not rsp.get("user_ids"):
break
if i >= max_pages:
raise RuntimeError("Too many pagination elements")
yield rsp["user_ids"]Зачем это условие с if i >= max_pages, просто убери в range +1 и все. оно же для каждой итерации будет выполняться...
def find(path: str, dict_: dict):
keys = path.split(".")
data = dict_
for key in keys:
try:
data = data.get(key)
except AttributeError:
return None
return databruh...
По поводу вкуса: полностью согласен. Тот же пример с sum можно и через [].count(True) решить. Но иногда бывает проще c пониманием того, что True + True == 2 . Хотел показать, что такой прием существует.
Про проверку на каждой итерации: добавил ее на случай, если мы хотим не просто молча оставить не обработанной часть результатов, а увидеть, что юзеров существует больше. В таком случае разработчик может пойти найти баг, либо ужесточить фильтры, либо элементарно увеличить max_pages
Сама проверка не ресурсозатраная, такое здорово оптимизируется, но в целом, можно обойтись и без нее
for i in range(max_pages + 1):
...
else:
print("too many")Правда, в таком варианте кода мы можем обработать на одну страницу больше, чем max_pages, затем только выкинуть, что страниц слишком много. Да и else после циклов даже для меня, любителя сахара, не прям читаем =)
Замечания полезные, благодарю!
А я за синтаксический сахар вообще бы убивал в детстве...
Хорошая статья, автор крут.
Я вот чего не знаю - if user := user_dict.get("user"):
Что значит двоеточие?
Благодарю!)
Это оператор присваивания в выражении. Подробнее
Его используют, чтобы не здавать переменной значение отдельно. В этом же примере без него пришлось бы делать так:
user = user_dict.get("user")
if user:
...# а так мы сразу и условие проверяем, и переменную заводим
if user := user_dict.get("user"):
...Не все любят этот оператор. Мне кажется, он подходит больше для простейших случаев, так как если вокруг него много скобок и логических операций, понимание затрудняется.
Ох уж этот листкопрехейшентрочизм. В питоне много можно писать в одну строчку и порой это интересно. Но далеко не всегда очевидно.
Прироста скорости не даёт. А разглядывать, что там наворочено в эту одну строчку то ещё удовольствие.
В связи с чем вопрос - зачем?
Не соглашусь насчет прироста скорости. Многие говорят о том, что компрехи быстрее.
Да и банальные тесты показывают эту разницу.
import time
iterations = 10000000
start = time.time()
mylist = []
for i in range(iterations):
mylist.append(i+1)
end = time.time()
print(end - start)
# 0.54514479637146
start = time.time()
mylist = [i+1 for i in range(iterations)]
end = time.time()
print(end - start)
# 0.32439470291137
Если по сравнению c filter, то да, так как она тоже быстрая. Хотя и встречал упоминания того, что компрехи весьма незначительно, но бывают быстрее. Но тут все же вопросы к читаемости лямбда-функций.
А разглядывать, что там наворочено в эту одну строчку то ещё удовольствие.
Безусловно, об этом не забыл упомянуть в статье. Если логика объемная, то генераторы списков того не стоят.
А если поменять местами в тесте и comprehension поставить первым?
прошел не один МР специалистов разного уровня.
А что такое МР?
Сорри, @lorerv это я случайно минус поставил, а отменить не могу. Вернул в карму.
Просто хотел ответить, что сравнение времен некорректное. list comprehension всегда медленнее, примерно на 10%. Это хотели исправить, встроив функцию генерации списка в текущий контекст в этом pep
А почему получилось медленнее в тесте - ответит любой олимпиадник: на For-Loop после каждых 20 добавленных элементов вызывается memory allocation на следующие 20 элементов. Это заметно жрет время. Потому для задач олимпиад, где есть требование по времени при работе с массивом известной длины можно встретить:
mylist = [] * iterations # резервирование памяти сразу под весь список
Небольшой офтоп, но касательно вашего примера. Я могу ошибаться, но мне кажется, что в классическом цикле, с эппэндом, скорость падает из-за этого самого эппэнда. Да, по факту компрехейш в этом случае оптимизирован лучше.
Но в этом случае можно идти дальше и рассматривать прочие механизмы оптимизации. Например, нампай. Плюс один импорт, но выше скорость вычислений.
Едва ли list comprehension со вложенными циклами улучшает код, хоть и позволяет сэкономить пару нажатий на клавиши. Кстати, в itertools есть функция product, которая помогает переписать пример из второго пункта вот так: [letter + number for letter, number in product(letters, numbers)]
Мне кажется это более читаемым, но это дело вкуса. А вот по поводу использования bool как int в документации кое-что написано: False and True behave like the integers 0 and 1, respectively. However, relying on this is discouraged; explicitly convert using int() instead. Тем не менее в тайп хинтах для функции sum специально прописали bool, так как это частый случай использования. Получаетсяsum([True, False]) - это не pythonic way?
Хороший коммент!
Я отталкивался от того, что используя двойной for именно в list comprehension, мы получаем плюшку в лице прироста производительности.
Вариант с product мне тоже очень понравился. Но при прочих равных предпочитаю конструкции, не требующие дополнительных импортов.
Про неочевидное поведение bool вопрос сложный. Наверное, оно само по себе протеворечит Дзену python, пункту "Явное лучше неявного".
Да, уменьшается количество кода в случаях типа sum([True, 1]) , но это действительно может обескуражить. Пожалуй, я просто уже привык к этой неявности и перестал воспринимать ее как таковую...
approvers = []
if approvers and approvers[-1].is_admin:
return True
return False
Можно ещё проще return approvers and approvers[-1].is_admin
Именно в таком варианте не совсем будет одно и то же.
approvers = []
def check(approvers: list) -> list | bool:
return approvers and approvers[-1].is_admin
check(approvers) # вернет []Лучше не мешать типы возвращаемых аргументов, так как это может привести к путанице.
Но эту конструкцию можно подправить до видаreturn bool(approvers and approvers[-1].is_admin)
Первые несколько пунктов статьи упустили главную проблему с булевскими переменными. Никто не обязан запоминать, а как в данном конкретном языке программирования, который исчезнет лет через 20, работает каст из чего угодно в булевскую переменную. Поэтому да, лучше использовать тернарники, лучше проверять списки на пустоту и лучше никогда не использовать единицу-ноль в качестве True-False. Это же Питон, а не С.
В 8 части ни слова про "ленивую" инициализацию, а ведь эта идея первична по отношению к генераторам.
Я много лет работаю консультантом на сложных Python проектах, вот дорос даже до доклада на нескольких конференциях и могу сказать, что именно упустил автор: Любое улучшение должно быть выполнено в стилистике кода проекта, а не в стилистике специалиста выполняюшего рефакторинг.
Как было упомянуто выше, тернарник или конкатенация проверочного условия через OR не имеют категорий "лучше-хуже". Есть четкая характеристика "используемая стилистика проекта", не важно, есть у вас отдельный StyleGuide или только код проекта, который, сам по себе, и есть StyleGuide. И предлагать чуждые проекту syntactic sugar конструкции - это ухудшить DevEx, который на большом проекте и так уже ниже плинтуса.
Кстати, при использовании тернарного оператора мы защищены от ошибки смены объекта в отличие от конкатенации условий:
в этом случае myobj будет указывать или на старый объект или на новый.
myobj = myobj or type(MyObj)()
А в этом случае указатель будет поменян только если myobj еще не был создан.
myobj = myobj if myobj is None else type(MyObj)()
Разумеется, MyObj специальный класс с переопределенным "__bool__" или "__eq__"
Про comprehensions я писал и рассказывал на PyCon: если итерироваться надо будет один раз, то это должен быть генератор. Если несколько раз - то, вероятно, выбран не тот алгоритм. В проектах, где я провожу code review, неоходимость размещения в памяти сразу всего сгенеренного массива разработчик должен аргументировать. Иначе, будь добр, ставь генератор.
Прежде чем рефакторить return True False в несколько строк, так же, сначала смотрим в код или styleguide.
Ну и про поиск в Словарях. Можно еще сделать так:
class DataDict:
def init(self, **kwargs):
self.dict.update({key: DataDict(**val) if isinstance (val, dict) else val for key, val in kwargs.items()})
def __getattr__(self, name):
return None
# где то в коде
value = DataDict(**data).user.permissions.create_usersВариант возврата Noneна несуществующий атрибут/ключ - это за гранью, поскольку нет возможности понять, значение ключа не передано или ключа нет вообще, но оставим это на совести атора.
Кстати <zanuda mode="on">:
В статье в примере этого раздела ошибка: при попытке получения в словаре несуществующего ключа быдет выброшен exception KeyError, а не AttributeError.
</zanuda>
Желаю автору успехов в работе с кодом. Ему это точно пригодится!
@lorerv не завершил предудущий коммент, а править уже нельзя.
Блок с названием "Генераторы для ..." и без генераторов. Вероятно, планировалось так:
def user_paginate(client):
limit, page = 3, 0
for page, users in enumerate(iter(lambda:client.get_all_users(page, limit))):
yield users
def user_limit_paginate(client, max_pages=3):
for page, users in enumerate(user_paginate(client), 1):
yield page, users
if page >= max_pages:
return
# gde to v code
all_paginated_users = yield from user_paginate(client)
paginated_users = yield from user_limit_paginate(client, 255)
# or with itertools, you are dont need user_limit_paginate function at all
import itertools
paginated_users = yield from itertools.batched(user_paginate(client), 255)
В первой функции я иду по итератору выдающему пачки users. уже позже понял что можно , что еще проще, если использовать generator.send(), но это оставим для следующей статьи.
Во второй функции я иду по итератору выдающему пачки users и стопаю итерацию после выдачи последнего допустимого user_chank. Хотя использовать itertools.batched в этом случае более предпочтительно.
Мод zanuda снимается. Не так прочел код. Там падение на .get
Но замечание про результат падения остается: при user.permissions.no_key.create_users непонятно, где выпал Еxception и выпал ли он. Я имею в виду, что None характеризует что (пустой словарь) or (нет user) or (user none) or (user пустой слолварь) or (user не содержит permissions) or ... продолжи сам.
Я исходил из того, что если мы хотим получать именно ошибку, то можно оставить стандартное поведение словаря, либо же использовать Pydantic для валидации.
Но часто при работе с нестрогими структурами, получаемыми по API, нам хватает условности, что несуществующий == None.
И тогда это может привести к коду, который был упомянут... С большим уровнем вложенности.
Спасибо, успех ни кому не помешает! =) Также благодарю за старательно написанные комментарии. Как раз подобные обсуждения и ценны.
По поводу словарей ответил ниже.
По тернарникам и условиям: именно поэтому и написал, что от него "можно уйти в некоторых случаях". Бывают ситуации, где коротенькое or смотрится гораздо лучше более громоздкого аналога. Посчитал, что начинающим кодерам не помешает знать о такой возможности.
С остальным согласен) особенно про приоритет стилистики проекта над стилистикой ревьюера.
P.S. Торжественно клянусь, что ни разу не заворачивал ревью из-за использования тернарника вместо or =)
часто изучают реализацию публичных популярных библиотек, сделанных профессионалами, чтобы изучать разные паттерны разработки. никогда такого не видел:
limit = 3
page = 0
while True:
rsp = client.get_all_users(page=page, limit=limit)
if not rsp.get("user_ids"):
break
yield rsp["user_ids"]
page += 1и вообще лучше написать понятно и просто, чем такие извращения. Сам потом забудешь, что имел ввиду
Но ради интересна в следующем проекте найду место для генератора
Улучшаем свой код на Python