Python умеет "из коробки" работать с zip. Можно удобно использовать контекстный менеджер with.

Называть открытый архив zip нельзя, так как это ключевое слово для одновременного итерирования по нескольким коллекциям. В этом случае добавляем нижнее подчёркивание в конце названия.

Подробнее о ZipFile в документации

#python #codereview

Давайте посмотрим на 10 строк кода.

Вроде всё хорошо. Классическое открытие файла с помощью контекстного менеджера with, классическая построчная обработка. Разбиваем строку, записываем в список словарей. Всё ли по канону? Неа:

1. Смотрим документацию: readlines возвращает список всех строк файла. Значит, для большого файла может быть беда. Современный питон позволяет итерироваться сразу по объекту file_. Просто пишем
for line in file_:

2. raw_line[0] и прочие индексы — это всегда ужас. Заменяем на
fio, login = line.split(";")

Вроде то же самое, но мы сразу понимаем, что было в строке. И тут же видим следующую проблему.

3. А что, если в строке нет двоеточия, или этих двоеточий больше одного? То есть нужна обработка ошибок на исключение ValueError, если справа split вернул не два значения. Добавляем try-except.

4. Частая проблема split — это лишние пробелы. Скорее всего, потребуется strip всем переменным после split.

Итого 4 ошибки на 10 строк кода

#python #codereview #devfm

Теперь посмотрим на переработанный код. Docstring вырезан для краткости. Код теперь крут:
1. Есть аннотация типов
2. С файлом работаем через контекстный менеджер, то есть закрыть не забудем — оно само
3. Не забыли про запрет использования file как ключевого слова, поэтому file_
4. При разбиении строки по точке с запятой используем именованные переменные
5. При ошибках — кастомные исключения. DownloadError название спорное, я бы заменил на ParsingError. Но если файл побился при скачивании, то самое то
6. Добавляем значения в список, не забывая обрезать лишние пробелы по бокам.

В общем, каждая строка на своём месте. Что можно улучшить? Я бы заменил result на logins, тогда и выходной словарик будет называться как надо.

#python #codereview #devfm

Словарь student (или result) во вчерашнем примере не является удобной конструкцией, ФИО доступно как student["fio"]. Кроме того, мы демонстрируем наружу внутреннее представление, нарушая принцип инкапсуляции. Замена словаря на список, например, заставит переписать весь код, который использует эту структуру данных. Какой может быть выход?

Создадим класс Student и превратим словарь в экземпляр класса. Можно использовать namedtuple из collections, но мы пойдём своим путём. Бонусов много:
1. Мы скрываем внутреннее устройство студента. Наружу мы отдаём только пару полей, откуда мы их берём никто снаружи не знает
2. Можем к студенту добавлять методы. Например, вывод фамилии с инициалами в стиле Иванов И.И. — теперь это в нашей власти
3. Можем добавить новые способы создания этого студента, например, данные брать из базы данных.

Пока методов нет, надо выключать диагностику pylint, а то нам будет ругаться " у класса слишком мало публичных методов". Не забываем включить её обратно после класса.

#python #codereview

В python есть неочевидные конструкции, например, else у цикла for. Часть с else выполнится, если цикл завершился сам, без break.

Хорошее применение — если есть два вложенных цикла (внимание! вычислительная сложность O(n^2), старайтесь избегать такого), и при завершении внутреннего надо завершить и внешний. Например, с помощью pandas разбираем эксельку с построчной обработкой, и в случае ошибок надо прекращать сразу оба цикла.

Если внутренний цикл прошёл целиком, то выполнится continue на строке 8, и внешний цикл продолжится. Если внутренний цикл на строке 5 завершится по break, то else не выполнится и мы попадём на строку 10 в break внешнего цикла.

Без for-else это решается флагом, который проверяется на внешнем цикле — то есть куда менее изящно.

Пример взят тут

#python #codereview

Рассмотрим простой фрагмент кода. Сохраняем список всех файлов и каталогов для "empty_dir" в переменную, потом проверяем, есть ли там что-то. Если что-то есть, то выводим список содержимого. Вроде всё корректно, но статический анализатор pylint даст вам по рукам с ошибкой

Do not use len(SEQUENCE) to determine if a sequence is empty (len-as-condition)

Достаточно непонятно написано. На самом деле, от вас требуется конструкция

if files:

Без всяких len. Потому что bool от сущности с нулевой длиной будет False. Такие дела.

#python #codereview

Ранее мы писали, что логгирование значительно лучше отладочной печати принтами.

Вверху сконфигурируем логгер. В fmt внесём структуру записи в лог, в datefmt — формат даты, в style — тип переменных в fmt, у нас выбран format-вид в фигурных скобках.

На 10 строке показан вариант ручной настройки уровня логгера, на 11 уровень берётся из переменной окружения LOGLEVEL, если переменная не существует — то уровень INFO.

В функции add_user мы с уровнем debug пишем какие-то несущественные сообщения, с уровнем info — важные этапы работы, в error пишем ошибки, например, в случае исключений.

Внизу пример вывода. В зависимости от переменной окружения у нас либо подробный лог, либо очень краткий. К каждому нашему сообщению автоматически (согласно fmt вверху) добавлена куча доп информации — уровень сообщения, дата и время, модуль, функция и строка, где сообщение было сформировано.

#python #codereview

Рассмотрим такой код. С помощью requests для заданного логина идём на gitlab.com и проверяем, что вернёт страничка пользователя. Если 200, то пользователь есть и возвращаем True. Если не 200, то пользователя нет или что-то сломалось.

Как это безобразие протестировать? Наш ответ в 22:00 по МСК

#python #codereview

Все варианты верны в той или иной степень. Тестировать код со внешними зависимостями действительно неприятно. Можно зафиксировать конкретные имена, что будет довольно грязно. Можно решить вопрос с помощью dependency injection и передать в функцию объект-requests, который умеет опрашивать сервер или имитировать подобное поведение.

Мы же воспользуемся заглушкой (Mock). Это механизм динамической замены функций/классов.

Создаём класс RequestsMock. Всё что он делает — это сохраняет переданный ему status_code.

Делаем with patch, и в этом блоке указанная функция (requests.get) будет подменена на подменный класс. Параметр side_effect многогранен, в нашем случае мы подаём туда список из объектов, и при каждом вызове requests.get будет возвращён очередной экземпляр из этого списка.

Теперь вместо реальной работы с сетью мы используем полностью контролируемое окружение. Можем так json-ы сервера подменять, например.

#python #codereview

Навигация по каналу

#sudo — наиболее важные посты. Начать знакомство с каналом рекомендуем с них.
#devfm — материалы собственного производства. Не просто аннотации, а наши мысли, статьи и видеоролики.

#python — фокусируемся на самом языке и его библиотеках.
#codereview — разбираем код, находим и устраняем проблемы, превращаем плохой код в хороший.
#procode — о профессиональной разработке и тестировании вне зависимости от языка.
#skills — о смежных с разработкой технических навыках, необходимых для работы и резюме. Инструменты (в том числе git, bash, docker), командная работа, безопасность и прочие фундаментальные вещи.
#systemdesign — проектирование систем и построение архитектуры.
#tools — полезные инструменты для работы.
#edu — полезные нетехнические навыки. Об обучении, продуктивности, английском, умении искать и обосновывать решения.
#youtube — видеоматериалы.
#fun — пятничное развлекательное и культурный код. Обзор художественных фильмов #films, книг #books, комиксов #xkcd и прочего.

#backup — лучшие посты месяца.

#codereview #quiz

Что же не так в этом коде?
В первую очередь опрос был на внимательность. Но есть и о чём порассуждать.
1. Сразу начнём с PEP8. Называть переменную List нельзя, такой формат именования используется для классов
2. Многие заметили, опечатку — создался List, срез делается по list. Но тут начинается интересное. Текущая версия кода вызовет ошибку TypeError: 'type' object is not subscriptable. Связано это с тем, что list — ключевое слово для создания списка в формате list(). Но без круглых скобок list — это сам класс list, а не экземпляр. У самого класса список не определена операция получения элемента по номеру (subscription).
3. Если код был бы такой
list_a = []
print(list_b[10:])
То мы получили бы ошибку NameError: name 'list_' is not defined.

Итого — TypeError, если мы использовали ключевое слово list и NameError, если мы использовали несуществующее название переменной. И ни одно из таких названий не должно пройти code review.

#codereview #quiz

В прошлый раз мы разобрались, как сделать удобный доступ к первому и последнему элементу объекта users. Для этого использовали динамические атрибуты и реализовали магический метод __getattr__.

При выполнении self.first = "new_user" происходит нежелательное поведение, и атрибут first перестанет указывать на первый элемент.

Для решения проблемы сделаем атрибуты first и second read only. Для этого определим магический метод __setattr__. Обратите внимание: __getattr__ вызывается, только если искомого атрибута нет, а метод __setattr__ вызывается каждый раз при присваивании значения атрибуту.

При попытке присвоить значение атрибуту проверяется содержится ли он в shortcut_names. Если содержится, то порождается исключение. Если не содержится, вызывается родительский __setattr__.
#codereview

Code review больше не будет прежним

Ревью кода в гитлабе сопряжено с неудобствами интерфейса merge request:
— видны только изменённые куски кода и не видно файла целиком. По сути, размывается одна из основных целей ревью кода — шаринг контекста.
— область просмотра кода маленькая из-за нагромождения всяких кнопочек самого гитлаба
— нет возможности запустить тесты, отладчик или сам код
— нет привычной навигации по проекту с использованием любовно настроенных шорткатов в IDE

Для меня всё перечисленное важно, поэтому раньше начинались танцы с бубном. В одном окне открыт pycharm с кодом, в другом — MR в гитлабе для замечаний.

Но есть супер удобный плагин gitlab merge request. Всё, что есть в вебе переезжает к вам в pycharm. Тут и удобно смотреть различия в коде, и видеть описание MRa, и оставлять комментарии, и отмечать уже просмотренные файлы.

НУ НАКОНЕЦ-ТО — подумал я, когда первый раз воспользовался этим плагином. Это то, что мы так долго ждали.

P.S. С некоторых пор плагин стал платным. Месяц бесплатно, потом 1.5$ в месяц или 15$ в год, но у многих могут быть проблемы с оплатой.
Рекомендую попробовать. Узнаете, как оно может быть удобно.
#devfm #codereview