feat(code quality): add black and ruff #56

zarifpour · 2023-06-01T04:34:19Z

I added black and ruff to maintain code quality and implemented a Makefile streamline the execution of common commands.

I also tried to add poetry as the dependency manager, but it was not compatible with the dependency openai-whisper. It seemed to consistently fail on the installation of tritton.

For reference, this is as far as I was able to get:

# pyproject.toml

[tool.poetry]
name = "ecoute"
version = "0.1.0"
description = "Ecoute is a live transcription tool that provides real-time transcripts for both the user's microphone input (You) and the user's speakers output (Speaker) in a textbox. It also generates a suggested response using OpenAI's GPT-3.5 for the user to say based on the live transcription of the conversation."
authors = ["Seva <[email protected]>"]
license = "MIT"
readme = "README.md"

[tool.poetry.dependencies]
python = "^3.8"
numpy = "==1.24.3"
openai-whisper = "==20230314"
Wave = "==0.0.2"
openai = "==0.27.6"
customtkinter = "==5.1.3"
torch = "==2.0.0"

[tool.poetry.dependencies.PyAudioWPatch]
version = "==0.2.12.5"
platform = "win32"

[tool.poetry.dependencies.pyaudio]
version = "*"
platform = "darwin"

# poetry replacement for: `--extra-index-url https://download.pytorch.org/whl/cu117`
[[tool.poetry.source]]
name = "torch"
url = "https://download.pytorch.org/whl/cu117"
priority = "supplemental"


[[tool.poetry.source]]
name = "PyPI"
priority = "primary"

[build-system]
requires = ["poetry-core"]
build-backend = "poetry.core.masonry.api"

It fails with this error (there was another one for tritton, but I seemed to have lost it somewhere in my console):

Package operations: 6 installs, 1 update, 0 removals

   • Updating torch (2.0.1 -> 2.0.0+cu117): Failed

   RuntimeError

   Unable to find installation candidates for torch (2.0.0+cu117)

   at ~/.pyenv/versions/3.11.1/lib/python3.11/site-packages/poetry/installation/chooser.py:76 in choose_for
        72│ 
        73│             links.append(link)
        74│ 
        75│         if not links:
   →  76│             raise RuntimeError(f"Unable to find installation candidates for {package}")
        77│ 
        78│         # Get the best link
        79│         chosen = max(links, key=lambda link: self._sort_key(package, link))
        80│

I noticed another developer had a similar issue: https://github.com/orgs/python-poetry/discussions/7199

SevaSk · 2023-06-01T19:26:43Z

Thanks for contributing! Could you split the PR between the streamlined installation changes and black and ruff changes? Want to consider those changes separately. Also I think we should ignore linting custom_speech_recognition since its just a copy of the existing SpeechRecognition library with some modifications. Probably a better way to maintain that I haven't considered how.

zarifpour · 2023-06-01T21:00:34Z

Thanks for contributing! Could you split the PR between the streamlined installation changes and black and ruff changes? Want to consider those changes separately. Also I think we should ignore linting custom_speech_recognition since its just a copy of the existing SpeechRecognition library with some modifications. Probably a better way to maintain that I haven't considered how.

I see, I didn't realize it was a copy of an existing SpeechRecognition library. In that case, I will ignore it in regards to linting and formatting. Would it make sense to do something like this - where the library is its own git repo?

Example from: https://github.com/foundry-rs/foundry-rust-template/tree/master/contracts/lib

zarifpour · 2023-06-01T21:20:09Z

Makefile

+lint:
+ @echo
+ ruff .
+ @echo
+ black --check --diff --color .
+ @echo
+ pip-audit
+
+format:
+ ruff --silent --exit-zero --fix .
+ black .
+
+precommit:
+ make lint
+ make format
+
+venv:
+ python3 -m venv ecout_env
+
+install:
+ pip install -r requirements.txt


These are the streamlined installation changes, let me know if it is necessary to split the PR.

zarifpour · 2023-06-01T21:20:20Z

README.md

+
+### Installation
+
+1. `make venv`
+2. Activate the venv: `ecout_venv`
+3. `make install`
+
+### Code quality
+
+Before submitting a pull request run `make precommit` and resolve any issues. Additionally, here are some useful commands:
+
+- `make lint`
+- `make format`


These are the streamlined installation changes, let me know if it is necessary to split the PR.

Oh Sry! Got confused with the Installation section above.

SevaSk · 2023-06-01T23:34:50Z

Makefile

+ make format
+
+venv:
+ python3 -m venv ecout_env


python or python3?

I think python3 is preferred since we are working with Python 3.x.x

Could somehow use either? For me I don't have python3 alias only python in my PATH.

Yeah, I can change it to python instead of python3.

SevaSk · 2023-06-01T23:36:14Z

Makefile

+ black .
+
+precommit:
+ make lint


Should probably have instructions to install 'make' as well, chocolatey seems easiest

I believe it's built-in on MacOS and Linux, not sure about Windows.

Windows you have to install it
installing choco with the commad in README and using

choco install make

would be good enough for instructions

SevaSk · 2023-06-01T23:39:12Z

README.md

+### Installation
+
+1. `make venv`
+2. Activate the venv: `ecout_venv`


ecout_env\Scripts\activate ?

I see, I'll add one script for MacOS and one for Windows.

Ah, sorry. I tried adding a script, but it's not possible to activate the venv with a make command. I'll add both options to the README: for MacOS it's source ecout_env/bin/activate

SevaSk · 2023-06-02T00:25:21Z

Not sure why but make lint and make format cant find python files

zarifpour · 2023-06-02T04:14:29Z

Not sure why but make lint and make format cant find python files

Hmm, I'm not sure. Maybe the way black behaves is different on windows. Can you try modifying a python file to contain single quotes for a string, instead of double quotes, and see if you get the same message. Perhaps it's behaving normally?

zarifpour · 2023-06-02T04:36:47Z

README.md

+#### Windows
+
+1. Install make on your Windows machine.
+
+ ```shell
+ choco install make
+ ```
+
+2. Create a virtual environment:
+
+ ```shell
+ make venv
+ ```
+
+3. Activate the virtual environment:
+
+ ```shell
+ .\ecout_venv\Scripts\activate
+ ```
+
+4. Install the required packages:
+
+ ```shell
+ make install
+ ```


Please confirm these steps work.

zarifpour · 2023-06-21T11:38:34Z

I think this is good to go. If you get a chance to review + merge, I would appreciate it. Thanks!

zarifpour added 2 commits June 1, 2023 00:23

feat: add black and ruff

0c5be21

Merge remote-tracking branch 'upstream/main'

6bf88b5

SevaSk self-requested a review June 1, 2023 19:16

zarifpour added 2 commits June 1, 2023 17:06

refactor(custom_speech_recognition): revert formatting

fff9765

fix(lint): ignore custom_speech_recognition

eb9d6ac

zarifpour commented Jun 1, 2023

View reviewed changes

SevaSk reviewed Jun 1, 2023

View reviewed changes

fix(OS compatibility): makefile & README

36162bd

zarifpour commented Jun 2, 2023

View reviewed changes

zarifpour added 2 commits June 2, 2023 00:40

docs: typo

4a27ea1

Merge branch 'main' into main

eadc4ca

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(code quality): add black and ruff #56

feat(code quality): add black and ruff #56

zarifpour commented Jun 1, 2023 •

edited

SevaSk commented Jun 1, 2023

zarifpour commented Jun 1, 2023

zarifpour Jun 1, 2023

zarifpour Jun 1, 2023

SevaSk Jun 1, 2023

SevaSk Jun 1, 2023

zarifpour Jun 1, 2023

SevaSk Jun 1, 2023

zarifpour Jun 2, 2023

zarifpour Jun 2, 2023

SevaSk Jun 1, 2023

zarifpour Jun 1, 2023

SevaSk Jun 2, 2023

SevaSk Jun 1, 2023

zarifpour Jun 1, 2023

zarifpour Jun 1, 2023

SevaSk commented Jun 2, 2023 •

edited

zarifpour commented Jun 2, 2023

zarifpour Jun 2, 2023

zarifpour commented Jun 21, 2023

feat(code quality): add black and ruff #56

Are you sure you want to change the base?

feat(code quality): add black and ruff #56

Conversation

zarifpour commented Jun 1, 2023 • edited

SevaSk commented Jun 1, 2023

zarifpour commented Jun 1, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

SevaSk commented Jun 2, 2023 • edited

zarifpour commented Jun 2, 2023

Choose a reason for hiding this comment

zarifpour commented Jun 21, 2023

zarifpour commented Jun 1, 2023 •

edited

SevaSk commented Jun 2, 2023 •

edited