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

pyproject.toml options with hyphens are ignored #502

Closed
dwt opened this issue Jun 26, 2023 · 7 comments · Fixed by #503 or akaihola/darkgraylib#23
Closed

pyproject.toml options with hyphens are ignored #502

dwt opened this issue Jun 26, 2023 · 7 comments · Fixed by #503 or akaihola/darkgraylib#23
Assignees
Labels
bug Something isn't working documentation Improvements or additions to documentation

Comments

@dwt
Copy link
Collaborator

dwt commented Jun 26, 2023

Describe the bug
It seems that Darker ignores config options in pyproject.toml if they are spelled using hyphens instead of underscores.

To Reproduce
I have configuration for a Darker pre-commit hook like this:

  - repo: https://github.com/akaihola/darker
    rev: 1.7.1
    hooks:
      - id: darker
        args: [ --skip-string-normalization ]

and a pyproject.toml like this:

[tool.darker]
line-length = 120
skip-string-normalization = true

However running pre-commit it did not honor the line length setting in pyproject.toml.

Reading the documentation and other bug reports here, it seems this ought to work.

Is this an issue in pre-commit, perhaps something to do with the fact that the pre-commit hook acts directly on the files that are part of the commit and therefore ignores the pyproject.toml?

Or is this a bug in Darker that I need to file there?

Changing the pre-commit hook to include the setting does seem to be a workaround. I.e.:

  - repo: https://github.com/akaihola/darker
    rev: 1.7.1
    hooks:
      - id: darker
        args: [ --skip-string-normalization , --line-length=120 ]

Expected behavior
Darker should honor the config option line-length = 120 in the pyproject.toml

Environment (please complete the following information):

  • OS: MacOS
  • Python version 3.10.12
  • Git version 2.39.2 (Apple Git-143)
  • Darker version 1.7.1
  • Black version black, 22.8.0 (compiled: yes)
  • other reformatter and linter versions ruff==0.0.272
@dwt
Copy link
Collaborator Author

dwt commented Jun 26, 2023

This is a continuation from pre-commit/pre-commit#2909 where I asked first, as I assumed this might be a feature of pre-commit that was misunderstanding.

@akaihola
Copy link
Owner

Thanks @dwt for the report, I was just able to reproduce this and will dig deeper now to find the cause.

@akaihola
Copy link
Owner

Actually @dwt I'm not sure if I really reproduced it.

Could you add verbosity to your configuration:

  - repo: https://github.com/akaihola/darker
    rev: 1.7.1
    hooks:
      - id: darker
        args: [ --skip-string-normalization, -vv ]

and run pre-commit again.

Do you see the line-length setting included in debug output for both the # Effective configuration: section and the # Configuration options which differ from defaults: section?

If you do see it, the configuration is read correctly from pyproject.toml. If you don't see it, please double check your pyproject.toml – I do see the setting if I add it.

I can continue with the investigation after you've tried this out.

@dwt
Copy link
Collaborator Author

dwt commented Jun 30, 2023

That's really strange.

With this diff:

❯ git diff --staged
diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml
index b7fd4f5..b55e32f 100644
--- a/.pre-commit-config.yaml
+++ b/.pre-commit-config.yaml
@@ -14,7 +14,8 @@ repos:
     rev: 1.7.1
     hooks:
       - id: darker
-        args: [ --skip-string-normalization , --line-length=120 ]
+        args: [ --skip-string-normalization , -vv ]
+        # args: [ --skip-string-normalization , --line-length=120 ]
 
   - repo: https://github.com/charliermarsh/ruff-pre-commit
     rev: "v0.0.274"
diff --git a/src/integration/api_db/tables.py b/src/integration/api_db/tables.py
index 64b7fc3..6c9caea 100644
--- a/src/integration/api_db/tables.py
+++ b/src/integration/api_db/tables.py
@@ -92,3 +92,5 @@ def db_save_to_tracing(path, request, response, korrelation_id=None, idempotency
         )
         conn.commit()
         return generated_id
+
+# fnord
\ No newline at end of file

I'm getting this output, suggesting that the line length limit of 120 is active:

❯ pre-commit
trim trailing whitespace.................................................Passed
fix end of files.........................................................Failed
- hook id: end-of-file-fixer
- exit code: 1
- files were modified by this hook

Fixing src/integration/api_db/tables.py

check yaml...............................................................Passed
check for added large files..............................................Passed
darker...................................................................Failed
- hook id: darker
- files were modified by this hook

# Effective configuration:

[tool.darker]
src = [
    "src/integration/api_db/tables.py",
]
revision = ":PRE-COMMIT:"
diff = false
stdout = false
check = false
flynt = false
isort = false
lint = [
]
log_level = "DEBUG"
skip_string_normalization = true
workers = 1
line-length = 120
skip-string-normalization = true


# Configuration options which differ from defaults:

[tool.darker]
src = [
    "src/integration/api_db/tables.py",
]
revision = ":PRE-COMMIT:"
log_level = "DEBUG"
skip_string_normalization = true
line-length = 120
skip-string-normalization = true



DEBUG:darker.git:[/Users/dwt/Code/Projekte/VBU/vbu-service/src/integration/api_db]$ git rev-parse --is-inside-work-tree
DEBUG:darker.git:[/Users/dwt/Code/Projekte/VBU/vbu-service/src/integration/api_db]$ git diff --name-only --relative HEAD -- tables.py
DEBUG:darker.git:[/Users/dwt/Code/Projekte/VBU/vbu-service/src/integration/api_db]$ git ls-files --others --exclude-standard -- tables.py
DEBUG:darker.__main__:Read 96 lines from edited file /Users/dwt/Code/Projekte/VBU/vbu-service/src/integration/api_db/tables.py
DEBUG:darker.__main__:Flynt resulted in 96 lines, with no changes from fstringification
DEBUG:darker.__main__:Black reformat resulted in 103 lines, with some changes from reformatting
DEBUG:darker.diff:Diff between edited and reformatted has 9 opcodes
DEBUG:darker.git:[/Users/dwt/Code/Projekte/VBU/vbu-service/src/integration/api_db]$ git show HEAD:./tables.py
DEBUG:darker.git:[/Users/dwt/Code/Projekte/VBU/vbu-service/src/integration/api_db]$ git log -1 --format=%ct HEAD -- tables.py
DEBUG:darker.diff:Diff between edited and reformatted has 2 opcodes
DEBUG:darker.chooser:Found no edits on lines 1-35
DEBUG:darker.chooser:Using 35 original lines at line 1
DEBUG:darker.chooser:Found no edits on line 36
DEBUG:darker.chooser:Using 1 original line at line 36
DEBUG:darker.chooser:Found no edits on lines 37-75
DEBUG:darker.chooser:Using 39 original lines at line 37
DEBUG:darker.chooser:Found no edits on lines 76-77
DEBUG:darker.chooser:Using 2 original lines at line 76
DEBUG:darker.chooser:Found no edits on lines 78-89
DEBUG:darker.chooser:Using 12 original lines at line 78
DEBUG:darker.chooser:Found no edits on line 90
DEBUG:darker.chooser:Using 1 original line at line 90
DEBUG:darker.chooser:Found edits on lines 91-95
DEBUG:darker.chooser:Using 5 unmodified lines at line 91
DEBUG:darker.chooser:Found edits on line 96
DEBUG:darker.chooser:Using 1 reformatted line at line 96
DEBUG:darker.chooser:Found edits on line 96
DEBUG:darker.chooser:Using 1 unmodified line at line 96
DEBUG:darker.__main__:AST verification success
INFO:darker.__main__:Writing 3033 bytes into /Users/dwt/Code/Projekte/VBU/vbu-service/src/integration/api_db/tables.py

ruff.....................................................................Passed

However, adding some example code to exercise the problem a) reveals a difference in behavior, and b) shows a surprising difference in verbose output.

Contrast this with the line-length setting only pyproject.toml:

❯ git diff --staged
diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml
index b7fd4f5..dd817eb 100644
--- a/.pre-commit-config.yaml
+++ b/.pre-commit-config.yaml
@@ -14,7 +14,8 @@ repos:
     rev: 1.7.1
     hooks:
       - id: darker
-        args: [ --skip-string-normalization , --line-length=120 ]
+        args: [ --skip-string-normalization , -vv ]
+        # args: [ --skip-string-normalization , --line-length=120 , -vv ]
 
   - repo: https://github.com/charliermarsh/ruff-pre-commit
     rev: "v0.0.274"
diff --git a/src/integration/api_db/tables.py b/src/integration/api_db/tables.py
index 64b7fc3..2a8a938 100644
--- a/src/integration/api_db/tables.py
+++ b/src/integration/api_db/tables.py
@@ -92,3 +92,15 @@ def db_save_to_tracing(path, request, response, korrelation_id=None, idempotency
         )
         conn.commit()
         return generated_id
+
+
+# fnord
+
+
+class D3:
+    def xxx(self, requests, url, headers, *, force: bool = False):
+        import sys
+
+        if None is None:
+            response = requests.request("GET", url, headers=headers, data=None, verify=False)
+            sys.stdout.write(response.text)

~/C/P/V/vbu-service  🐍 vbu-api ⛵️ k8s-deep-dive (mh) 🌱 infra/update-local-crdb $+  
❯ pre-commit
trim trailing whitespace.................................................Passed
fix end of files.........................................................Passed
check yaml...............................................................Passed
check for added large files..............................................Passed
darker...................................................................Failed
- hook id: darker
- files were modified by this hook

# Effective configuration:

[tool.darker]
src = [
    "src/integration/api_db/tables.py",
]
revision = ":PRE-COMMIT:"
diff = false
stdout = false
check = false
flynt = false
isort = false
lint = [
]
log_level = "DEBUG"
skip_string_normalization = true
workers = 1
line-length = 120
skip-string-normalization = true


# Configuration options which differ from defaults:

[tool.darker]
src = [
    "src/integration/api_db/tables.py",
]
revision = ":PRE-COMMIT:"
log_level = "DEBUG"
skip_string_normalization = true
line-length = 120
skip-string-normalization = true



DEBUG:darker.git:[/Users/dwt/Code/Projekte/VBU/vbu-service/src/integration/api_db]$ git rev-parse --is-inside-work-tree
DEBUG:darker.git:[/Users/dwt/Code/Projekte/VBU/vbu-service/src/integration/api_db]$ git diff --name-only --relative HEAD -- tables.py
DEBUG:darker.git:[/Users/dwt/Code/Projekte/VBU/vbu-service/src/integration/api_db]$ git ls-files --others --exclude-standard -- tables.py
DEBUG:darker.__main__:Read 106 lines from edited file /Users/dwt/Code/Projekte/VBU/vbu-service/src/integration/api_db/tables.py
DEBUG:darker.__main__:Flynt resulted in 106 lines, with no changes from fstringification
DEBUG:darker.__main__:Black reformat resulted in 114 lines, with some changes from reformatting
DEBUG:darker.diff:Diff between edited and reformatted has 9 opcodes
DEBUG:darker.git:[/Users/dwt/Code/Projekte/VBU/vbu-service/src/integration/api_db]$ git show HEAD:./tables.py
DEBUG:darker.git:[/Users/dwt/Code/Projekte/VBU/vbu-service/src/integration/api_db]$ git log -1 --format=%ct HEAD -- tables.py
DEBUG:darker.diff:Diff between edited and reformatted has 2 opcodes
DEBUG:darker.chooser:Found no edits on lines 1-35
DEBUG:darker.chooser:Using 35 original lines at line 1
DEBUG:darker.chooser:Found no edits on line 36
DEBUG:darker.chooser:Using 1 original line at line 36
DEBUG:darker.chooser:Found no edits on lines 37-75
DEBUG:darker.chooser:Using 39 original lines at line 37
DEBUG:darker.chooser:Found no edits on lines 76-77
DEBUG:darker.chooser:Using 2 original lines at line 76
DEBUG:darker.chooser:Found no edits on lines 78-89
DEBUG:darker.chooser:Using 12 original lines at line 78
DEBUG:darker.chooser:Found no edits on line 90
DEBUG:darker.chooser:Using 1 original line at line 90
DEBUG:darker.chooser:Found edits on lines 91-104
DEBUG:darker.chooser:Using 14 unmodified lines at line 91
DEBUG:darker.chooser:Found edits on line 105
DEBUG:darker.chooser:Using 3 reformatted lines at line 105
DEBUG:darker.chooser:Found edits on line 106
DEBUG:darker.chooser:Using 1 unmodified line at line 106
DEBUG:darker.__main__:AST verification success
INFO:darker.__main__:Writing 3325 bytes into /Users/dwt/Code/Projekte/VBU/vbu-service/src/integration/api_db/tables.py

ruff.....................................................................Passed

~/C/P/V/vbu-service  🐍 vbu-api ⛵️ k8s-deep-dive (mh) 🌱 infra/update-local-crdb $!+  
❯ git diff
diff --git a/src/integration/api_db/tables.py b/src/integration/api_db/tables.py
index 2a8a938..c9ab489 100644
--- a/src/integration/api_db/tables.py
+++ b/src/integration/api_db/tables.py
@@ -102,5 +102,7 @@ class D3:
         import sys
 
         if None is None:
-            response = requests.request("GET", url, headers=headers, data=None, verify=False)
+            response = requests.request(
+                "GET", url, headers=headers, data=None, verify=False
+            )
             sys.stdout.write(response.text)

which produces this diff above, suggesting that the line-length setting is in fact not honored, while this invocation:

❯ git diff --staged
diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml
index b7fd4f5..6dd28cc 100644
--- a/.pre-commit-config.yaml
+++ b/.pre-commit-config.yaml
@@ -14,7 +14,8 @@ repos:
     rev: 1.7.1
     hooks:
       - id: darker
-        args: [ --skip-string-normalization , --line-length=120 ]
+        # args: [ --skip-string-normalization , -vv ]
+        args: [ --skip-string-normalization , --line-length=120 , -vv ]
 
   - repo: https://github.com/charliermarsh/ruff-pre-commit
     rev: "v0.0.274"
diff --git a/src/integration/api_db/tables.py b/src/integration/api_db/tables.py
index 64b7fc3..20ec993 100644
--- a/src/integration/api_db/tables.py
+++ b/src/integration/api_db/tables.py
@@ -92,3 +92,12 @@ def db_save_to_tracing(path, request, response, korrelation_id=None, idempotency
         )
         conn.commit()
         return generated_id
+
+
+# fnord
+class D3:
+    def xxx(self, requests, url, headers, *, force: bool = False):
+        import sys
+        if None is None:
+            response = requests.request("GET", url, headers=headers, data=None, verify=False)
+            sys.stdout.write(response.text)

~/C/P/V/vbu-service  🐍 vbu-api ⛵️ k8s-deep-dive (mh) 🌱 infra/update-local-crdb $+  
❯ pre-commit
trim trailing whitespace.................................................Passed
fix end of files.........................................................Passed
check yaml...............................................................Passed
check for added large files..............................................Passed
darker...................................................................Failed
- hook id: darker
- files were modified by this hook

# Effective configuration:

[tool.darker]
src = [
    "src/integration/api_db/tables.py",
]
revision = ":PRE-COMMIT:"
diff = false
stdout = false
check = false
flynt = false
isort = false
lint = [
]
log_level = "DEBUG"
skip_string_normalization = true
line_length = 120
workers = 1
line-length = 120
skip-string-normalization = true


# Configuration options which differ from defaults:

[tool.darker]
src = [
    "src/integration/api_db/tables.py",
]
revision = ":PRE-COMMIT:"
log_level = "DEBUG"
skip_string_normalization = true
line_length = 120
line-length = 120
skip-string-normalization = true



DEBUG:darker.git:[/Users/dwt/Code/Projekte/VBU/vbu-service/src/integration/api_db]$ git rev-parse --is-inside-work-tree
DEBUG:darker.git:[/Users/dwt/Code/Projekte/VBU/vbu-service/src/integration/api_db]$ git diff --name-only --relative HEAD -- tables.py
DEBUG:darker.git:[/Users/dwt/Code/Projekte/VBU/vbu-service/src/integration/api_db]$ git ls-files --others --exclude-standard -- tables.py
DEBUG:darker.__main__:Read 103 lines from edited file /Users/dwt/Code/Projekte/VBU/vbu-service/src/integration/api_db/tables.py
DEBUG:darker.__main__:Flynt resulted in 103 lines, with no changes from fstringification
DEBUG:darker.__main__:Black reformat resulted in 110 lines, with some changes from reformatting
DEBUG:darker.diff:Diff between edited and reformatted has 9 opcodes
DEBUG:darker.git:[/Users/dwt/Code/Projekte/VBU/vbu-service/src/integration/api_db]$ git show HEAD:./tables.py
DEBUG:darker.git:[/Users/dwt/Code/Projekte/VBU/vbu-service/src/integration/api_db]$ git log -1 --format=%ct HEAD -- tables.py
DEBUG:darker.diff:Diff between edited and reformatted has 2 opcodes
DEBUG:darker.chooser:Found no edits on lines 1-35
DEBUG:darker.chooser:Using 35 original lines at line 1
DEBUG:darker.chooser:Found no edits on line 36
DEBUG:darker.chooser:Using 1 original line at line 36
DEBUG:darker.chooser:Found no edits on lines 37-75
DEBUG:darker.chooser:Using 39 original lines at line 37
DEBUG:darker.chooser:Found no edits on lines 76-77
DEBUG:darker.chooser:Using 2 original lines at line 76
DEBUG:darker.chooser:Found no edits on lines 78-89
DEBUG:darker.chooser:Using 12 original lines at line 78
DEBUG:darker.chooser:Found no edits on line 90
DEBUG:darker.chooser:Using 1 original line at line 90
DEBUG:darker.chooser:Found edits on lines 91-100
DEBUG:darker.chooser:Using 10 unmodified lines at line 91
DEBUG:darker.chooser:Found edits on line 101
DEBUG:darker.chooser:Using 1 reformatted line at line 101
DEBUG:darker.chooser:Found edits on lines 101-103
DEBUG:darker.chooser:Using 3 unmodified lines at line 101
DEBUG:darker.__main__:AST verification success
INFO:darker.__main__:Writing 3293 bytes into /Users/dwt/Code/Projekte/VBU/vbu-service/src/integration/api_db/tables.py

ruff.....................................................................Passed

~/C/P/V/vbu-service  🐍 vbu-api ⛵️ k8s-deep-dive (mh) 🌱 infra/update-local-crdb $!+  
❯ git diff
diff --git a/src/integration/api_db/tables.py b/src/integration/api_db/tables.py
index 20ec993..a316bbe 100644
--- a/src/integration/api_db/tables.py
+++ b/src/integration/api_db/tables.py
@@ -98,6 +98,7 @@ def db_save_to_tracing(path, request, response, korrelation_id=None, idempotency
 class D3:
     def xxx(self, requests, url, headers, *, force: bool = False):
         import sys
+
         if None is None:
             response = requests.request("GET", url, headers=headers, data=None, verify=False)
             sys.stdout.write(response.text)

on the one had seems to honor the line-length setting and produces this curious output:

# Effective configuration:

[tool.darker]
src = [
    "src/integration/api_db/tables.py",
]
revision = ":PRE-COMMIT:"
diff = false
stdout = false
check = false
flynt = false
isort = false
lint = [
]
log_level = "DEBUG"
skip_string_normalization = true
line_length = 120
workers = 1
line-length = 120
skip-string-normalization = true


# Configuration options which differ from defaults:

[tool.darker]
src = [
    "src/integration/api_db/tables.py",
]
revision = ":PRE-COMMIT:"
log_level = "DEBUG"
skip_string_normalization = true
line_length = 120
line-length = 120
skip-string-normalization = true

Might it be, that there is an identifier confusion between line-length and line_length?

@akaihola
Copy link
Owner

This indeed reveals a bug. line-length in pyproject.toml is output as line-length in the dumped configuration at debug log level. But the --line-length command line argument is output as line_length instead.

Probably the bug isn't limited to just the configuration dump, but also is reflected in how the option (and probably other options) are taken into account when Darker is run.

But this would mean that it should be easy to replicate the behavior you see without involving pre-commit. Could you try doing that, i.e. running Darker on the command line instead and using the same pyproject.toml and command line options from pre-commit args: configuration?

I'll look closer into configuration and command line parsing next.

@akaihola
Copy link
Owner

akaihola commented Jun 30, 2023

I tested this on the command line with the following results:

  • --line-length=120 on the command line works correctly and shows as line_length = 120 in the configuration dump
  • --line_length=120 on the command line causes an error
  • line_length = 120 in pyproject.toml works correctly and shows as line_length = 120 in the configuration dump
  • line-length = 120 in pyproject.toml doesn't work and shows as line-length = 120 in the configuration dump
  • the value on the command line always overrides the one in pyproject.toml

I can see two alternatives for a fix:

  1. make sure all documentation and tests use underscores for pyproject.toml, and raise an error if hyphens are found instead (this is what isort seems to do)
  2. convert hyphens to underscores when parsing pyproject.toml (this is what black seems to do)

I chose to implement alternative 2. As a bonus, the debug log configuration dump normalizes option keys to use only hyphens.
Does that make good sense to you?

@akaihola akaihola self-assigned this Jun 30, 2023
@akaihola akaihola added bug Something isn't working documentation Improvements or additions to documentation labels Jun 30, 2023
@akaihola akaihola added this to the Darker 1.7.2 milestone Jun 30, 2023
@akaihola akaihola changed the title darker pre-commit hook seems to ignore configuration in pyproject.toml pyproject.toml options with hyphens are ignored Jun 30, 2023
@dwt
Copy link
Collaborator Author

dwt commented Jul 1, 2023

That sure makes sense to me.

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working documentation Improvements or additions to documentation
Projects
2 participants