Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Планы по улучшению документации #44

Open
SerGeRybakov opened this issue Nov 16, 2024 · 6 comments
Open

Планы по улучшению документации #44

SerGeRybakov opened this issue Nov 16, 2024 · 6 comments
Assignees
Labels
documentation Improvements or additions to documentation enhancement New feature or request

Comments

@SerGeRybakov
Copy link

Привет! Спасибо за обалденную ШТУКУ!

Огорчают два момента:

  1. не очень внятная документация и отсуствие хороших докстрингов к классам и методам. Из документации на readthedocs я так и не смог понять, что, например, делает метод CounterToken.keep_on(), а также почему этот метод либо доступ к CounterToken.cancelled, или вызов CounterToken.is_cancelled() уменьшают счётчик.
  2. наличие у конечных объектов большого множества публичных методов родителей, назначение и необходимость публичности которых остаётся не до конца ясными по причине первого пункта.

Есть планы это поправить?

@SerGeRybakov SerGeRybakov added the documentation Improvements or additions to documentation label Nov 16, 2024
@pomponchik
Copy link
Owner

pomponchik commented Nov 16, 2024

Привет! Спасибо за интерес к проекту.

Давай я немного поменяю нумерацию в твоем исходном сообщении, чтобы было проще отвечать:

  1. Отсутствие докстрингов к классам и методам.
  2. Недостаточная документация на readthedocs в целом.
  3. Конкретно отсутствие инфы про декремент счетчика у CounterToken в тех трех случаях.
  4. Публичны методы, которые не должны быть публичными.

Теперь поехали по порядку:

  1. Тут согласен, их стоило бы добавить, по крайней мере на те классы и методы, которые упоминаются в документации, т.е. являются частью публичного API. Когда конкретно у меня дойдут руки это сделать - сказать сейчас не могу, т.к. подобные изменения мне видятся вторичными на фоне тех, что непосредственно влияют на функциональность. Если есть желание поработать над этим, приглашаю начать.

  2. Если есть еще примеры, просьба присылать, будем фиксить.

  3. Применительно к токенам в целом назначение указанных методов описано в соответствующем разделе документации. Конкретно по декременту счетчика у CounterToken также есть некоторое описание:

CounterToken is initialized with an integer greater than zero. At each calculation of the answer to the question whether it is canceled, this number is reduced by one. When this number becomes zero, the token is considered canceled:

[some code]

The counter inside the CounterToken is reduced under one of three conditions:

Access to the cancelled attribute.
Calling the is_cancelled() method.
Calling the keep_on() method.

На русский это можно перевести примерно так:

CounterToken инициализируется целым числом больше нуля. При каждом вычислении ответа на вопрос, отменено ли это, это число уменьшается на единицу. Когда это число становится равным нулю, токен считается отмененным:

[пример кода]

Счетчик внутри CounterToken уменьшается при одном из трех условий:

Доступ к cancelled атрибуту.
Вызов is_cancelled() метода.
Вызов keep_on() метода.

Лично мне этот текст кажется достаточно подробным и объясняющим суть того, что происходит при вызове соответствующих методов. Если со стороны так не кажется, просьба конкретизировать, чего именно не хватает.

  1. Лично я не особый фанат "сокрытия" методов в питоновском стиле. Дело в том, что python не предоставляет истинного сокрытия, а то сокрытие, которое мы получаем путем модификации имен методов - легко обходится любым желающим. От пользователей я ожидаю, что они будут использовать те методы, которые увидят в документации, а значит им не грозит использовать метод, которому надлежит быть скрытым. От тех же, кто по какой-то причине решил идти в обход документации, я ожидаю способности самостоятельно разобраться в том, как все работает. То есть сокрытие будто бы не дает никаких плюсов, вводя при этом налог на чтение и написание кода - в моем понимании код, перенасыщенный вызовами "сокрытых" методов воспринимается сложнее.

Если есть аргументы, почему я не прав - готов обсудить. Мое мнение могли бы изменить, скажем, аргументы, связанные с работой подсказок в популярных IDE.

@pomponchik pomponchik added help wanted Extra attention is needed and removed documentation Improvements or additions to documentation labels Nov 17, 2024
@SerGeRybakov
Copy link
Author

Да, именно из-за подсветки синтаксиса я тебе и написал.

Смотри, как я понял как этим пользоваться (поправь меня, если я ошибаюсь): к созданному токену мы обращаемся либо для проверки его состояния, чтобы принять какое-то решение, либо отменяем его вручную по какому-то кейсу, в остальное же время на него вообще не нужно обращать внимания.

Если же дёрнуть любой токен за точку, то в IDE (лично я пользуюсь PyCharm) на тебя вываливается список из кучи методов, с которыми совершенно не понятно что делать (в частности, потому что у них нет никакого описания внутри).

К сожалению, я не смог сделать скрин, да на него бы и не влезло всё, но я не поленился и просто по точке всё выписал. Вот, что мы имеем от IDE, доступившись к токену:

t = SimpleToken()

t.cached_report
t.cancel()
t.cancelled
t.check()
t.check_superpower()
t.check_superpower_with_rollback()
t.exception
t.filter_tokens()
t.get_extra_kwargs()
t.get_report()
t.get_superpower_data()
t.get_superpower_exception_message()
t.is_cancelled()
t.keep_on()
t.lock
t.raise_cancelled_exception()
t.raise_superpower_exception()
t.rollback_if_nondirect_polling
t.superpower()
t.superpower_rollback()
t.text_representation_of_extra_kwargs()
t.text_representation_of_kwargs()
t.text_representation_of_superpower()
t.tokens
t.wait()

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

Например, в документации ты явным образом пишешь про keep_on(), что его вообще трогать не надо, типа оно и без него само по себе отлично работает.

You don't have to call the keep_on() method directly. Use the token itself as a boolean value, and the method call will occur "under the hood" automatically:

Тогда возникает логичный вопрос: с какой целью он публичный? Почему бы тогда не попрятать подобные вещи в protected, чтобы они могли наследоваться, если они нужны под капотом, или вообще их не выпилить?

По поводу предложения поработать над документацией, я искренне с радостью бы взялся, если бы понимал на 100% всю логику, а главное задумку и посыл. Но пока что к сожалению не понимаю. Мне очень нравится эта штука, и я очень благодарен, что ты её вытащил для нас на свет. Если меня внезапно осенит или посетит муза, я непременно пришлю тебе PR. Ну или мы перейдём в более приватное общение, и я у тебя буду долго и нудно выпытывать "что ты вот тут имел в виду" ). Но для начала я бы разобрался с компоновкой публичного API.

@pomponchik pomponchik added documentation Improvements or additions to documentation enhancement New feature or request and removed help wanted Extra attention is needed labels Nov 18, 2024
@pomponchik
Copy link
Owner

pomponchik commented Nov 18, 2024

Да, ты прав, нужно будет сделать часть методов и атрибутов приватными. Займусь этим как будет время. Надумаешь сам пописать доку - велком либо тут, либо в личку.

@SerGeRybakov
Copy link
Author

Вот, запилил #45

@SerGeRybakov
Copy link
Author

ЗЫ: это конечно пока только набросок и мёржить это нельзя

@pomponchik
Copy link
Owner

Ага, спасибо, я покомментил там. Думаю, на базе твоего PR попробую тоже что-то сделать в ближайшее время.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants