-
-
Notifications
You must be signed in to change notification settings - Fork 148
fixup package to use pyproject.toml #100
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
base: master
Are you sure you want to change the base?
Conversation
- remove setup.py and use optional-dependencies for audio, pip install video-to-ascii[audio]
WalkthroughThe pull request updates the installation instructions by replacing Changes
Sequence Diagram(s)sequenceDiagram
participant User as User
participant Setup as setup.py
participant Setuptools as setuptools
User->>Setup: Run installation command
Setup->>Setuptools: Call setup(extras_require={"audio": ["pyaudio"]})
Setuptools-->>User: Manage dependency installation (optional audio)
Poem
✨ Finishing Touches
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (3)
setup.py (1)
1-8
: Consider removing or aligning this minimal setup.py.
Since you’ve already defined project metadata inpyproject.toml
, this minimalsetup.py
is somewhat redundant. You can either remove it entirely if you’re adopting a pure PEP 517 approach, or align the extras (e.g., enforce the same version constraint “>=0.2.14” forpyaudio
as specified in yourpyproject.toml
) to avoid installing different versions unexpectedly.README.md (2)
41-41
: Remove the leading dollar sign or show the command output.
Markdown lint rules (MD014) suggest removing the$
prompt unless you’re also showing output. For consistency with standard practice, consider removing it to avoid linter warnings.- $ pip install video-to-ascii + pip install video-to-ascii🧰 Tools
🪛 markdownlint-cli2 (0.17.2)
41-41: Dollar signs used before commands without showing output
null(MD014, commands-show-output)
47-47
: Consider removing the$
prompt for the audio installation command as well.
Similarly, removing the$
sign or showing example output will eliminate the lint warning and keep the README consistent.- $ pip install video-to-ascii[audio] # or "video-to-ascii[audio]" on zsh + pip install video-to-ascii[audio] # or "video-to-ascii[audio]" on zsh🧰 Tools
🪛 markdownlint-cli2 (0.17.2)
47-47: Dollar signs used before commands without showing output
null(MD014, commands-show-output)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
README.md
(1 hunks)pyproject.toml
(1 hunks)setup.py
(1 hunks)
🧰 Additional context used
🪛 markdownlint-cli2 (0.17.2)
README.md
41-41: Dollar signs used before commands without showing output
null
(MD014, commands-show-output)
47-47: Dollar signs used before commands without showing output
null
(MD014, commands-show-output)
🔇 Additional comments (1)
pyproject.toml (1)
1-44
: Excellent transition to modern Python packaging.
Thispyproject.toml
follows recommended best practices, clearly specifying build requirements, project metadata, dependencies, optional dependencies, and scripts. If you want absolutely reproducible builds, consider pinning dependency versions more strictly. Otherwise, this setup is well-structured and aligns with current standards.
fixes #94
The current setup.py depends on no-longer supported pip internals. This change updates to the common pyproject.toml format which should preserve all of the previous behavior moving the prior
--install-option="--with-audio"
to extras by defining the project's optional-dependencies foraudio
so now you can justpip install video-to-ascii[audio]
.Summary by CodeRabbit
Documentation
Chores