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

comma added in embetter/text/__init_.py fixes #101 #103

Merged
merged 1 commit into from
Apr 26, 2024

Conversation

anopsy
Copy link
Contributor

@anopsy anopsy commented Apr 26, 2024

fixed the comma
btw
I was curious about the ruff pre-commit hooks. You have that in sklego, I wondered why not here. I mean if there is a reason or just not yet. If not yet, then that might be something I would like to try to do.

@koaning
Copy link
Owner

koaning commented Apr 26, 2024

Francesco added ruff on sklego. Given that I have a busy busy busy life I prefer not to make changes to my repos unless I really need to. Flake8 works fine, ruff is just a bit faster.

Thanks for the PR!

@koaning koaning merged commit 8f7603f into koaning:main Apr 26, 2024
3 checks passed
@anopsy
Copy link
Contributor Author

anopsy commented Apr 26, 2024

Oh I see. Btw, do I see correctly that the unit tests failed on this PR?
=========================== short test summary info ============================
FAILED tests/test_text.py::test_basic_bpemb[both] - UnicodeDecodeError: 'utf-8' codec can't decode byte 0xbf in position 40762: invalid start byte

I run the tests in pytests and all passed, before committing. Also checked the files locally with Black. I wonder what happened here?

@koaning
Copy link
Owner

koaning commented Apr 26, 2024

The checks look green on your PR, no?

CleanShot 2024-04-26 at 22 27 10@2x

@anopsy
Copy link
Contributor Author

anopsy commented Apr 26, 2024

I could have screenshoted it, I saw red on unittest, but now it's green. Now I wonder even more. 😅 Anyway, I'm happy it's all good now😊

@anopsy
Copy link
Contributor Author

anopsy commented Apr 27, 2024

Francesco added ruff on sklego. Given that I have a busy busy busy life I prefer not to make changes to my repos unless I really need to. Flake8 works fine, ruff is just a bit faster.

Just to clarify, I meant the lack of pre-commits in general. I can't see neither ruff nor Flake8 as the pre-commit hooks. When I was committing:

  • I didn't see any checks from flake8 happening (unless it's happening under the hood) ,
  • Flake8 didn't catch that missing comma,
  • I also don't see the pre-commit-config.yaml in the repo. I see there's .flake8 file, but I'm not sure at what point he checks the code.

@koaning
Copy link
Owner

koaning commented Apr 27, 2024

Ah like that! I think the main reason why these are missing is because the embetter repo really only has one author (me) and I've sofar managed fine by running most of these things locally. Locally I usually do that by running make check which runs all of my checks.

You'd certainly be welcome to add these to the repo if you'd enjoy the exercise by the way, it's just that this is one of the plugins that I made in a hurry and never really took the effort to make the project more appropriate for a "multi-author" situation. Scikit-lego was multi-author from the start so we took more time into the developer experience there.

@anopsy
Copy link
Contributor Author

anopsy commented Apr 27, 2024

yeah I noticed that you're the only contributor and thought that probably embetter was a just a little convenient tool. Anyway I would love to add the hooks. Thank you clarifying

@anopsy
Copy link
Contributor Author

anopsy commented May 1, 2024

what do we want? flake8, black or ruff?

@koaning
Copy link
Owner

koaning commented May 1, 2024 via email

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants