ShadPython - 2 - Ryzhikov Artem - CS Novosibirsk

ShadPython - 2 - Ryzhikov Artem - CS Novosibirsk
ID - 713780

[shad.python, 2015-06-25 09:55:06]

https://codereview.appspot.com/245440043/diff/1/main.py
File main.py (right):

https://codereview.appspot.com/245440043/diff/1/main.py#newcode14
main.py:14: def setup_console(sys_enc="utf-8"):
Думаю, проблему с кодировками можно решить и проще - для этого нужно аккуратно
работать в коде с юникодом и кодировками.

Общая идея такая. Сразу после получения входного текста (который будет в str)
нужно его раскодировать и он превратится в unicode. Далее в коде нужно везде
работать с unicode, а потом перед выводом опять закодировать (и он превратится в
str). Функция print по-хорошему не должна уметь выводить на экран unicode (как
вы делаете), но некоторые системы умеют это делать (по-разному, отсюда и
проблемы).

Таким образом в коде нужно убедиться, что после перекодирования вы везде
работаете с unicode (например, в с. 81 вы делаете strip unicode-строки по
str-строке), а везде перед выводом делать выполнять encode.

https://codereview.appspot.com/245440043/diff/1/main.py#newcode39
main.py:39: # returns probabilities of chains
Этот текст лучше переместить в докстринг функции, чтобы его, например, могли
извлекать текстовые редакторы. Еще такие доксринги в питоне принято писать в
повелительном наклонении.

def get_probabilities(...):

    """Return probabilities of chains."""

    ...

https://codereview.appspot.com/245440043/diff/1/main.py#newcode46
main.py:46: words = words + get_tokens(line, with_signs=False)
Я бы рекомендовал сделать, чтобы эта функция сразу получала на вход
правильно отфильтрованный список/генератор токенов, а не сама их
делала/фильтровала (возможно, в зависимости от сложных опций).
Тогда, если нужно будет использовать эту функцию в новой задаче, ее не
придется переделывать.

https://codereview.appspot.com/245440043/diff/1/main.py#newcode180
main.py:180: next_words_map = get_probabilities(lines, max_key_depth=depth,
max_value_depth=depth)
Не очень хорошо, что функция обучения модели (сбора вероятностей) находится
прямо в функции генерации. Для того, чтобы сгенерировать несколько текстов на
основе одной модели, придется каждый раз заново обучать ее. Лучше разделить эти
действия.

[artemryzhikov, 2015-06-25 18:05:47]

https://codereview.appspot.com/245440043/diff/1/main.py
File main.py (right):

https://codereview.appspot.com/245440043/diff/1/main.py#newcode14
main.py:14: def setup_console(sys_enc="utf-8"):
On 2015/06/25 09:55:06, shad.python wrote:
> Думаю, проблему с кодировками можно решить и проще - для этого нужно аккуратно
> работать в коде с юникодом и кодировками.
> 
> Общая идея такая. Сразу после получения входного текста (который будет в str)
> нужно его раскодировать и он превратится в unicode. Далее в коде нужно везде
> работать с unicode, а потом перед выводом опять закодировать (и он превратится
в
> str). Функция print по-хорошему не должна уметь выводить на экран unicode (как
> вы делаете), но некоторые системы умеют это делать (по-разному, отсюда и
> проблемы).
> 
> Таким образом в коде нужно убедиться, что после перекодирования вы везде
> работаете с unicode (например, в с. 81 вы делаете strip unicode-строки по
> str-строке), а везде перед выводом делать выполнять encode.

Done.

https://codereview.appspot.com/245440043/diff/1/main.py#newcode39
main.py:39: # returns probabilities of chains
On 2015/06/25 09:55:06, shad.python wrote:
> Этот текст лучше переместить в докстринг функции, чтобы его, например, могли
> извлекать текстовые редакторы. Еще такие доксринги в питоне принято писать в
> повелительном наклонении.
> 
> def get_probabilities(...):
> 
>     """Return probabilities of chains."""
> 
>     ...

Done.

https://codereview.appspot.com/245440043/diff/1/main.py#newcode46
main.py:46: words = words + get_tokens(line, with_signs=False)
On 2015/06/25 09:55:06, shad.python wrote:
> Я бы рекомендовал сделать, чтобы эта функция сразу получала на вход
> правильно отфильтрованный список/генератор токенов, а не сама их
> делала/фильтровала (возможно, в зависимости от сложных опций).
> Тогда, если нужно будет использовать эту функцию в новой задаче, ее не
> придется переделывать.

Done.

https://codereview.appspot.com/245440043/diff/1/main.py#newcode180
main.py:180: next_words_map = get_probabilities(lines, max_key_depth=depth,
max_value_depth=depth)
On 2015/06/25 09:55:06, shad.python wrote:
> Не очень хорошо, что функция обучения модели (сбора вероятностей) находится
> прямо в функции генерации. Для того, чтобы сгенерировать несколько текстов на
> основе одной модели, придется каждый раз заново обучать ее. Лучше разделить
эти
> действия.

Done.