-
Notifications
You must be signed in to change notification settings - Fork 87
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
chore(deps-dev): bump black, bump flake8, remove unused dev-dependencies #742
Conversation
New and removed dependencies detected. Learn more about Socket for GitHub ↗︎
🚮 Removed packages: pypi/[email protected], pypi/[email protected], pypi/[email protected], pypi/[email protected], pypi/[email protected], pypi/[email protected], pypi/[email protected], pypi/[email protected] |
@dependabot rebase |
fa3fc53
to
96b0723
Compare
This PR will fail until someone runs |
96b0723
to
f03866d
Compare
Important Review skippedBot user detected. To trigger a single review, invoke the You can disable this status message by setting the 🪧 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 (
|
a206c6d
to
1bb42e6
Compare
Bumps [black](https://github.com/psf/black) from 23.3.0 to 24.8.0. - [Release notes](https://github.com/psf/black/releases) - [Changelog](https://github.com/psf/black/blob/main/CHANGES.md) - [Commits](psf/black@23.3.0...24.8.0) --- updated-dependencies: - dependency-name: black dependency-type: direct:development update-type: version-update:semver-major ... Signed-off-by: dependabot[bot] <[email protected]>
1bb42e6
to
b0ea3a1
Compare
@mvadari In my local system, I don't observe these errors, if black is executed with the following environment:
Can we upgrade the version of Python and Poetry to more recent values? Is it necessary to set the CI/CD environment to the minimum supported Python version? |
Sorry, I remembered the wrong syntax - just
I observe the errors with these settings:
We shouldn't remove support for old Python versions without a need. Why remove backwards compatibility if you don't have to? |
@mvadari I'm unable to replicate the behavior of the CI/CD step. I don't get the
As far as I see it, these environment variables are identical to the CI/CD system.(Except for the MacOS and arm64 architecture) |
pyproject.toml
Outdated
@@ -23,7 +23,7 @@ include = ["LICENSE"] | |||
packages = [{ include = "xrpl" }] | |||
|
|||
[tool.poetry.dependencies] | |||
python = "^3.8" | |||
python = ">=3.8.1,<3.14" |
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.
Why is there a max number here?
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.
One of the dependencies darglint
has an upper bound on the supported Python version. Here is the error message:
The current project's supported Python range (>=3.8.1) is not compatible with some of the required packages Python requirement:
- darglint requires Python >=3.6,<4.0, so it will not be satisfied for Python >=4.0
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.
Here's the complete error log:
➜ xrpl-py git:(dependabot/pip/black-24.8.0) poetry lock
Updating dependencies
Resolving dependencies... (2.1s)
The current project's supported Python range (>=3.8.1) is not compatible with some of the required packages Python requirement:
- darglint requires Python >=3.6,<4.0, so it will not be satisfied for Python >=4.0
- darglint requires Python >=3.6,<4.0, so it will not be satisfied for Python >=4.0
- darglint requires Python >=3.6,<4.0, so it will not be satisfied for Python >=4.0
- darglint requires Python >=3.6,<4.0, so it will not be satisfied for Python >=4.0
- darglint requires Python >=3.6,<4.0, so it will not be satisfied for Python >=4.0
Because no versions of darglint match >1.5.8,<1.6.0 || >1.6.0,<1.7.0 || >1.7.0,<1.8.0 || >1.8.0,<1.8.1 || >1.8.1,<2.0.0
and darglint (1.5.8) requires Python >=3.6,<4.0, darglint is forbidden.
And because darglint (1.6.0) requires Python >=3.6,<4.0, darglint is forbidden.
And because darglint (1.7.0) requires Python >=3.6,<4.0
and darglint (1.8.0) requires Python >=3.6,<4.0, darglint is forbidden.
So, because darglint (1.8.1) requires Python >=3.6,<4.0
and xrpl-py depends on darglint (^1.5.8), version solving failed.
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.
I have a PR replacing darglint, so this is probably a moot point: #749
It's a very outdated package.
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.
This can include <4.0
, but it shouldn't be gating minor releases. Most of them work automatically (see #753), so gating it would actually be worse/require more work, since this would prevent it from working automatically without a version bump.
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.
fixed in 43fc875
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.
I haven't yet looked at the darglint PR. We could revisit the gating upper bound later.
@@ -34,14 +34,13 @@ types-Deprecated = "^1.2.9" | |||
pycryptodome = "^3.16.0" | |||
|
|||
[tool.poetry.dev-dependencies] | |||
flake8 = "^4.0.1" | |||
black = "23.3.0" | |||
flake8 = "^7.0.0" |
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.
PR title should be updated with this as well
This reverts commit 8ba6c03.
flake8-black = "^0.3.6" | ||
flake8-docstrings = "^1.7.0" | ||
mypy = "^1" | ||
isort = "^5.11.5" | ||
flake8-isort = "^6.0.0" | ||
flake8-annotations = "2.7.0" |
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.
Why is this removed?
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.
I have updated the rationale for the removal in the description of the PR.
flake8-annotations depends on an older version of flake8. The solution is to upgrade flake8-annotations to a higher version.
But I couldn't find any usages of this dependency. Where is it used?
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.
It isn't used directly, it's used by flake8 to check function annotations: https://github.com/sco1/flake8-annotations?tab=readme-ov-file#table-of-warnings
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.
thanks. updated in 563897f
flake8-absolute-import = "^1.0" | ||
darglint = "^1.5.8" | ||
sphinx-rtd-theme = "^3.0.0" | ||
aiounittest = "^1.4.0" | ||
coverage = "^7.2.7" | ||
Jinja2 = "^3.1.4" | ||
MarkupSafe = "2.1.5" |
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.
Why is this removed?
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.
I've updated the rationale in the description of the PR.
I couldn't find any usages of this dependency. Where is it used?
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.
It's a dependency for Jinja2. I'm not sure why Jinja2 is included though tbh, maybe it was previously a dependency for something.
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.
Jinja2
is also not needed. I'm happy to remove it.
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.
If nothing complains about it, sure
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.
removed in 563897f
I didn't find any usage of Older rippled API versions are indicated by explicit version numbers, rather than a |
I believe all the |
Are there valid uses for the |
It's used for deprecated xrpl-py things, not deprecated rippled things. |
This reverts commit 02fc14a.
A summary of the manual updates to this PR:
[email protected]
and the existing version offlake8
dependency. I suspect this is because we are using an old version offlake8
.flake8
to the latest version which was released 3 months ago. Release History: https://pypi.org/project/flake8/#historyflake8-annotations
is not compatible with[email protected]
. Ideally, we will need to upgrade it to a compatible version. But that would increase the size of changes in this PR. Removing this dependency has not broken any of the tests. I believe it does not break any functionality of the library either.MarkupSafe
dev-dependency (https://pypi.org/project/MarkupSafe/) is not used inside the xrpl-py client library. I'd have preferred to remove the dead-dependencies in a separate PR, but this PR was frozen due to an old poetry.lock file after this commit: 0b11443. I took this opportunity to clean up the unused dependency.black
automatically.Bumps black from 23.3.0 to 24.8.0.
Release notes
Sourced from black's releases.
... (truncated)
Changelog
Sourced from black's changelog.
... (truncated)
Commits
b965c2a
Prepare release 24.8.0 (#4426)9ccf279
Documentfind_project_root
ignoringpyproject.toml
without[tool.black]
...14b6e61
fix: Enhace black efficiently to skip directories listed in .gitignore (#4415)b1c4dd9
fix: respect braces better in f-string parsing (#4422)4b4ae43
Fix incorrect linenos on fstring tokens with escaped newlines (#4423)7fa1faf
docs: fix the installation command of extra for blackd (#4413)8827acc
Bump sphinx from 7.3.7 to 7.4.0 in /docs (#4404)b0da11d
Bump furo from 2024.5.6 to 2024.7.18 in /docs (#4409)721dff5
fix: avoid formatting backslash strings inside f-strings (#4401)7e2afc9
Updateactions/checkout
to v4 to stop node deprecation warnings (#4379)You can trigger a rebase of this PR by commenting
@dependabot rebase
.Dependabot commands and options
You can trigger Dependabot actions by commenting on this PR:
@dependabot rebase
will rebase this PR@dependabot recreate
will recreate this PR, overwriting any edits that have been made to it@dependabot merge
will merge this PR after your CI passes on it@dependabot squash and merge
will squash and merge this PR after your CI passes on it@dependabot cancel merge
will cancel a previously requested merge and block automerging@dependabot reopen
will reopen this PR if it is closed@dependabot close
will close this PR and stop Dependabot recreating it. You can achieve the same result by closing it manually@dependabot show <dependency name> ignore conditions
will show all of the ignore conditions of the specified dependency@dependabot ignore this major version
will close this PR and stop Dependabot creating any more for this major version (unless you reopen the PR or upgrade to it yourself)@dependabot ignore this minor version
will close this PR and stop Dependabot creating any more for this minor version (unless you reopen the PR or upgrade to it yourself)@dependabot ignore this dependency
will close this PR and stop Dependabot creating any more for this dependency (unless you reopen the PR or upgrade to it yourself)