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

chore(deps-dev): bump black, bump flake8, remove unused dev-dependencies #742

Merged
merged 16 commits into from
Nov 11, 2024

Conversation

dependabot[bot]
Copy link
Contributor

@dependabot dependabot bot commented on behalf of github Sep 1, 2024

A summary of the manual updates to this PR:

  • In my local setup, I observed a conflict between the behavior of [email protected] and the existing version of flake8 dependency. I suspect this is because we are using an old version of flake8.
  • I have updated flake8 to the latest version which was released 3 months ago. Release History: https://pypi.org/project/flake8/#history
  • The existing version of the dependency flake8-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.
  • On similar lines, 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.
  • As a result of the black version upgrade, certain files had to undergo minor changes. Please check the last 3 commits for those changes. All the other linter changes were performed by black automatically.

Bumps black from 23.3.0 to 24.8.0.

Release notes

Sourced from black's releases.

24.8.0

Stable style

  • Fix crash when # fmt: off is used before a closing parenthesis or bracket. (#4363)

Packaging

  • Packaging metadata updated: docs are explictly linked, the issue tracker is now also linked. This improves the PyPI listing for Black. (#4345)

Parser

  • Fix regression where Black failed to parse a multiline f-string containing another multiline string (#4339)
  • Fix regression where Black failed to parse an escaped single quote inside an f-string (#4401)
  • Fix bug with Black incorrectly parsing empty lines with a backslash (#4343)
  • Fix bugs with Black's tokenizer not handling \{ inside f-strings very well (#4422)
  • Fix incorrect line numbers in the tokenizer for certain tokens within f-strings (#4423)

Performance

  • Improve performance when a large directory is listed in .gitignore (#4415)

Blackd

  • Fix blackd (and all extras installs) for docker container (#4357)

24.4.2

This is a bugfix release to fix two regressions in the new f-string parser introduced in 24.4.1.

Parser

  • Fix regression where certain complex f-strings failed to parse (#4332)

Performance

  • Fix bad performance on certain complex string literals (#4331)

24.4.1

Highlights

  • Add support for the new Python 3.12 f-string syntax introduced by PEP 701 (#3822)

Stable style

  • Fix crash involving indented dummy functions containing newlines (#4318)

... (truncated)

Changelog

Sourced from black's changelog.

24.8.0

Stable style

  • Fix crash when # fmt: off is used before a closing parenthesis or bracket. (#4363)

Packaging

  • Packaging metadata updated: docs are explictly linked, the issue tracker is now also linked. This improves the PyPI listing for Black. (#4345)

Parser

  • Fix regression where Black failed to parse a multiline f-string containing another multiline string (#4339)
  • Fix regression where Black failed to parse an escaped single quote inside an f-string (#4401)
  • Fix bug with Black incorrectly parsing empty lines with a backslash (#4343)
  • Fix bugs with Black's tokenizer not handling \{ inside f-strings very well (#4422)
  • Fix incorrect line numbers in the tokenizer for certain tokens within f-strings (#4423)

Performance

  • Improve performance when a large directory is listed in .gitignore (#4415)

Blackd

  • Fix blackd (and all extras installs) for docker container (#4357)

24.4.2

This is a bugfix release to fix two regressions in the new f-string parser introduced in 24.4.1.

Parser

  • Fix regression where certain complex f-strings failed to parse (#4332)

Performance

  • Fix bad performance on certain complex string literals (#4331)

24.4.1

Highlights

  • Add support for the new Python 3.12 f-string syntax introduced by PEP 701 (#3822)

Stable style

... (truncated)

Commits
  • b965c2a Prepare release 24.8.0 (#4426)
  • 9ccf279 Document find_project_root ignoring pyproject.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 Update actions/checkout to v4 to stop node deprecation warnings (#4379)
  • Additional commits viewable in compare view

Dependabot compatibility score

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)

Note
Automatic rebases have been disabled on this pull request as it has been open for over 30 days.

@dependabot dependabot bot added the dependencies Pull requests that update a dependency file label Sep 1, 2024
@ckeshava
Copy link
Collaborator

@dependabot rebase

@dependabot dependabot bot force-pushed the dependabot/pip/black-24.8.0 branch from fa3fc53 to 96b0723 Compare September 26, 2024 18:02
@mvadari
Copy link
Collaborator

mvadari commented Sep 26, 2024

This PR will fail until someone runs black --fix on all the changes

@dependabot dependabot bot force-pushed the dependabot/pip/black-24.8.0 branch from 96b0723 to f03866d Compare September 26, 2024 18:24
Copy link
Contributor

coderabbitai bot commented Sep 26, 2024

Important

Review skipped

Bot user detected.

To trigger a single review, invoke the @coderabbitai review command.

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.


🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Generate unit testing code for this file.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai generate unit testing code for this file.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and generate unit testing code.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

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)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

@dependabot dependabot bot force-pushed the dependabot/pip/black-24.8.0 branch 2 times, most recently from a206c6d to 1bb42e6 Compare September 26, 2024 18:49
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]>
@dependabot dependabot bot force-pushed the dependabot/pip/black-24.8.0 branch from 1bb42e6 to b0ea3a1 Compare September 26, 2024 19:33
@ckeshava
Copy link
Collaborator

ckeshava commented Nov 4, 2024

@mvadari black does not have a command line option titled --fix. As per their recent release notes, black does not support Python 3.8 anymore.

In my local system, I don't observe these errors, if black is executed with the following environment:

➜  xrpl-py git:(dependabot/pip/black-24.8.0) python --version
Python 3.11.6
➜  xrpl-py git:(dependabot/pip/black-24.8.0) poetry --version
Poetry (version 1.8.3)

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?

@mvadari
Copy link
Collaborator

mvadari commented Nov 4, 2024

black does not have a command line option titled --fix. As per their recent release notes, black does not support Python 3.8 anymore.

Sorry, I remembered the wrong syntax - just poetry run black xrpl will fix everything.

In my local system, I don't observe these errors, if black is executed with the following environment:

➜  xrpl-py git:(dependabot/pip/black-24.8.0) python --version
Python 3.11.6
➜  xrpl-py git:(dependabot/pip/black-24.8.0) poetry --version
Poetry (version 1.8.3)

I observe the errors with these settings:

xrpl-py % poetry run python --version
Python 3.11.4
xrpl-py % poetry --version
Poetry (version 1.8.4)

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?

We shouldn't remove support for old Python versions without a need. Why remove backwards compatibility if you don't have to?

@ckeshava
Copy link
Collaborator

ckeshava commented Nov 5, 2024

@mvadari I'm unable to replicate the behavior of the CI/CD step. I don't get the BLK100 error message, despite having identical environment installation.

➜  xrpl-py git:(dependabot/pip/black-24.8.0) poetry run flake8 xrpl tests snippets --darglint-ignore-regex="^_(.*)"
➜  xrpl-py git:(dependabot/pip/black-24.8.0) black --version                                                       
black, 24.8.0 (compiled: yes)
Python (CPython) 3.8.18
➜  xrpl-py git:(dependabot/pip/black-24.8.0) python --version
Python 3.8.18
➜  xrpl-py git:(dependabot/pip/black-24.8.0) poetry --version
Poetry (version 1.8.3)
➜  xrpl-py git:(dependabot/pip/black-24.8.0) poetry run flake8 xrpl tests snippets --darglint-ignore-regex="^_(.*)"
➜  xrpl-py git:(dependabot/pip/black-24.8.0) 

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"
Copy link
Collaborator

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?

Copy link
Collaborator

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

Copy link
Collaborator

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.

Copy link
Collaborator

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.

Copy link
Collaborator

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.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fixed in 43fc875

Copy link
Collaborator

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"
Copy link
Collaborator

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

@ckeshava ckeshava changed the title chore(deps-dev): bump black from 23.3.0 to 24.8.0 chore(deps-dev): bump black from 23.3.0 to 24.8.0, update flake8 to v7.0.0 Nov 6, 2024
@ckeshava ckeshava changed the title chore(deps-dev): bump black from 23.3.0 to 24.8.0, update flake8 to v7.0.0 chore(deps-dev): bump black from 23.3.0 to 24.8.0, update flake8 to v7.0.0, remove unused dev-dependencies Nov 6, 2024
@ckeshava ckeshava requested a review from mvadari November 6, 2024 22:40
@mvadari mvadari changed the title chore(deps-dev): bump black from 23.3.0 to 24.8.0, update flake8 to v7.0.0, remove unused dev-dependencies chore(deps-dev): bump black, bump flake8, remove unused dev-dependencies Nov 6, 2024
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"
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is this removed?

Copy link
Collaborator

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?

Copy link
Collaborator

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

Copy link
Collaborator

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"
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is this removed?

Copy link
Collaborator

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?

Copy link
Collaborator

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.

Copy link
Collaborator

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.

Copy link
Collaborator

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

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

removed in 563897f

@ckeshava
Copy link
Collaborator

ckeshava commented Nov 7, 2024

I didn't find any usage of @deprecated decorator in the codebase. As indicated in this commit: 02fc14a, we can remove these two dependencies.

Older rippled API versions are indicated by explicit version numbers, rather than a deprecated tag.

@ckeshava ckeshava requested a review from mvadari November 7, 2024 21:12
@ckeshava ckeshava mentioned this pull request Nov 7, 2024
9 tasks
@mvadari
Copy link
Collaborator

mvadari commented Nov 7, 2024

I didn't find any usage of @deprecated decorator in the codebase. As indicated in this commit: 02fc14a, we can remove these two dependencies.

Older rippled API versions are indicated by explicit version numbers, rather than a deprecated tag.

I believe all the @deprecated functions were removed. IMO we shouldn't remove the dependencies, in case we want to deprecate functions in the future.

@ckeshava
Copy link
Collaborator

ckeshava commented Nov 7, 2024

I believe all the @deprecated functions were removed. IMO we shouldn't remove the dependencies, in case we want to deprecate functions in the future.

Are there valid uses for the @deprecated decorator? Won't we gate the "deprecated" methods under older API versions? Why would we need this dependency?

@mvadari
Copy link
Collaborator

mvadari commented Nov 7, 2024

Are there valid uses for the @deprecated decorator? Won't we gate the "deprecated" methods under older API versions? Why would we need this dependency?

It's used for deprecated xrpl-py things, not deprecated rippled things.

@justinr1234 justinr1234 merged commit 4649643 into main Nov 11, 2024
22 checks passed
@justinr1234 justinr1234 deleted the dependabot/pip/black-24.8.0 branch November 11, 2024 18:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dependencies Pull requests that update a dependency file
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants