milharal-dev / nemli-nemlerei-bot

Um bot de discord para criar resumos baseado nas últimas mensagens de um canal
8 stars 1 forks source link

DX: pre-commit #24

Closed mattpsvreis closed 1 month ago

mattpsvreis commented 1 month ago

What it does: image

Addresses:

ryukinix commented 1 month ago

Eu preferia que houvesse uma função como essa na própria lib, algo como um módulo onde está essa nemli.setup. Possa ser usada no nemli.main:run e também nos testes. Pode continuar com a ideia de fixtures, mas era melhor que houvesse apenas um lugar de implementação

Em qua., 24 de jul. de 2024 08:38, ⟠ Rodolfo De Nadai < @.***> escreveu:

@.**** requested changes on this pull request.

Essa minha branch pode ajudar nisso tudo...

https://github.com/milharal-dev/nemli-nemlerei-bot/tree/rdenadai/pre-commit

In tests/test_stopwords.py https://github.com/milharal-dev/nemli-nemlerei-bot/pull/24#discussion_r1689600174 :

@@ -2,6 +2,22 @@

from nemli.nlp.messages import clean_up_stopwords

+import nltk + + +def setup_nltk():

  • try:
  • nltk.data.find("corpora/stopwords")
  • except LookupError:
  • nltk.download("stopwords")
  • try:
  • nltk.data.find("tokenizers/punkt")
  • except LookupError:
  • nltk.download("punkt")
  • +setup_nltk()

Suggestion:

pytest fixture é a melhor opção para este caso.

Cria um arquivo conftest.py no diretório de tests e coloca uma fixture como por exemplo:

import pytestfrom nltk import download as nltk_download

@pytest.fixture()def setup_nltk_stopwords(): nltk_download("punkt") # Download nltk punkt nltk_download("stopwords") # Download nltk stopwords

E depois só colocar ela como param na assinatura da função de teste... deve funcionar.

def test_clean_up_stopwords(setup_nltk_stopwords, message: str, expected: str):


In .pre-commit-config.yaml https://github.com/milharal-dev/nemli-nemlerei-bot/pull/24#discussion_r1689613701 :

@@ -0,0 +1,10 @@ +repos:

Suggestion: Por que não algo como?

repos:

Sugiro adicionar o black --check como comando para o pdm check assim verifica tb se formatação esta ok.

— Reply to this email directly, view it on GitHub https://github.com/milharal-dev/nemli-nemlerei-bot/pull/24#pullrequestreview-2196402152, or unsubscribe https://github.com/notifications/unsubscribe-auth/AB2J57TT6AMF5DFDIUAKRODZN6G3LAVCNFSM6AAAAABLLXAG5WVHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMZDCOJWGQYDEMJVGI . You are receiving this because your review was requested.Message ID: @.***>

rdenadai commented 1 month ago

Eu preferia que houvesse uma função como essa na própria lib, algo como um módulo onde está essa nemli.setup. Possa ser usada no nemli.main:run e também nos testes. Pode continuar com a ideia de fixtures, mas era melhor que houvesse apenas um lugar de implementação

A fixture pode chamar esse método ... sem problemas. Mencionei a fixture, pq se vc rodar os testes individualmente não precisaria ficar executando esse setup para todos eles, execeto quando vc rodasse o teste q usa a fixture.