| Message ID | 20210802181359.10695-1-goodoldpaul@autistici.org |
|---|---|
| State | Accepted |
| Headers | show |
| Series | [bug#49834,1/7] gnu: Add python-flake8-debugger. | expand |
| Context | Check | Description |
|---|---|---|
| cbaines/comparison | success | View comparision |
| cbaines/git branch | success | View Git branch |
| cbaines/applying patch | fail | View Laminar job |
| cbaines/issue | success | View issue |
Hello again, Thanks for the revised patches. I was able to apply your patches and take a closer look, and found a few more things. (Sorry!) Giacomo Leidi <goodoldpaul@autistici.org> writes: > * gnu/packages/python-xyz.scm (python-flake8-debugger): New variable. > --- > gnu/packages/python-xyz.scm | 27 ++++++++++++++++++++++++++- > 1 file changed, 26 insertions(+), 1 deletion(-) > > diff --git a/gnu/packages/python-xyz.scm b/gnu/packages/python-xyz.scm > index 61db9febb8..fc7f08a720 100644 > --- a/gnu/packages/python-xyz.scm > +++ b/gnu/packages/python-xyz.scm > @@ -64,7 +64,7 @@ > ;;; Copyright © 2019, 2020 Alex Griffin <a@ajgrf.com> > ;;; Copyright © 2019, 2020 Pierre Langlois <pierre.langlois@gmx.com> > ;;; Copyright © 2019 Jacob MacDonald <jaccarmac@gmail.com> > -;;; Copyright © 2019, 2020 Giacomo Leidi <goodoldpaul@autistici.org> > +;;; Copyright © 2019, 2020, 2021 Giacomo Leidi <goodoldpaul@autistici.org> > ;;; Copyright © 2019 Wiktor Żelazny <wzelazny@vurv.cz> > ;;; Copyright © 2019, 2020, 2021 Tanguy Le Carrour <tanguy@bioneland.org> > ;;; Copyright © 2019, 2021 Mădălin Ionel Patrașcu <madalinionel.patrascu@mdc-berlin.de> > @@ -9456,6 +9456,31 @@ These should be used in preference to using a backslash for line continuation. > @end quotation") > (license license:asl2.0))) > > +(define-public python-flake8-debugger > + (package > + (name "python-flake8-debugger") > + (version "4.0.0") > + (source > + (origin > + (method url-fetch) > + (uri (pypi-uri "flake8-debugger" version)) > + (sha256 > + (base32 > + "19pdfx0rb3k1i8hxfjdi8ndd6ayzq8g1041j8zdq256vyxvwfgg4")))) > + (build-system python-build-system) > + (propagated-inputs > + `(("python-flake8" ,python-flake8) > + ("python-pycodestyle" ,python-pycodestyle) > + ("python-six" ,python-six))) > + (home-page > + "https://github.com/jbkahn/flake8-debugger") > + (synopsis > + "Ipdb/pdb statement checker plugin for flake8") > + (description > + "This package provides the @code{flake8-debugger} Python module, > +an ipdb/pdb statement checker plugin for flake8.") > + (license license:expat))) The pyproject.toml file does say this is "MIT" (= expat), but there's no actual license file in the project. I don't think this is sufficient as far as Guix is concerned (perhaps an actual maintainer/committer can comment?) > + > (define-public python-flake8-implicit-str-concat > (package > (name "python-flake8-implicit-str-concat") > > base-commit: f12a35cfa22092a7e3157c94abfef8335f86ac1c -- Sarah
Dear Sarah, thank you for your review! On 8/4/21 8:26 PM, Sarah Morgensen wrote: > The pyproject.toml file does say this is "MIT" (= expat), but there's no > actual license file in the project. I don't think this is sufficient as > far as Guix is concerned (perhaps an actual maintainer/committer can > comment?) First of all I opened an upstream PR [0] to add an actual LICENSE file: I hope that, given its simplicity, it will get merged soon. I'm no expert about this but I tried building dynaconf without flake8-debugger and it seems to work flawlessly. The dependency was put there by the python importer, probably the developers have it in some dev-dependencies declaration needed to commit well-formatted code in the repository. Since flake8-debugger doesn't appear to be required for the inclusion of dynaconf maybe we can exclude it for this time and whenever the PR will be merged I'll open another Guix issue to add it. What do you think about this approach? (I'm attaching an updated patch-set without flake8-debugger and rebased on current master). Thank you for your time :) Giacomo [0]: https://github.com/JBKahn/flake8-debugger/pull/29/files
Hi Giacomo, Giacomo Leidi <goodoldpaul@autistici.org> writes: > Dear Sarah, > > thank you for your review! Thank you for your continued work on this. I know it can be a long and sometimes discouraging process, and we often end up with abandoned patches because of it. > > On 8/4/21 8:26 PM, Sarah Morgensen wrote: >> The pyproject.toml file does say this is "MIT" (= expat), but there's no >> actual license file in the project. I don't think this is sufficient as >> far as Guix is concerned (perhaps an actual maintainer/committer can >> comment?) > > First of all I opened an upstream PR [0] to add an actual LICENSE file: I hope > that, given its simplicity, it will get merged soon. Fingers crossed! > > I'm no expert about this but I tried building dynaconf without flake8-debugger > and it seems to work flawlessly. The dependency was put there by the python > importer, probably the developers have it in some dev-dependencies declaration > needed to commit well-formatted code in the repository. > > Since flake8-debugger doesn't appear to be required for the inclusion of > dynaconf maybe we can exclude it for this time and whenever the PR will > be merged I'll open another Guix issue to add it. > > What do you think about this approach? (I'm attaching an updated patch-set > without flake8-debugger and rebased on current master). Did you receive my other email? I found that in fact none of the flake8 packages, as well as some others, are actually required. I apologise if merging these bugs caused it to get lost! I'll quote it below. Sarah Morgensen <iskarian@mgsn.dev> writes: > Hi, > > Thanks again for your work on packaging this! > > Giacomo Leidi <goodoldpaul@autistici.org> writes: > >> * gnu/packages/python-xyz.scm (python-colorama-0.4.1): New variable, >> (python-dotenv-0.13.0): New variable, >> (dynaconf): New variable. > > Packages typically get one commit per package (so this would be three > commits). > >> * gnu/packages/patches/dynaconf-Unvendor-dependencies.patch: New file. > ^ an extra space slipped in here. > >> [...] >> + (arguments >> + `(#:phases >> + (modify-phases %standard-phases >> + (replace 'check >> + (lambda* (#:key tests? outputs #:allow-other-keys) >> + (when tests? >> + (setenv "PATH" >> + (string-append (assoc-ref outputs "out") "/bin:" >> + (getenv "PATH"))) >> + ;; These tests depend on hvac and a >> + ;; live Vault process. >> + (delete-file "tests/test_vault.py") >> + (invoke "make" "test_only")) >> + #t))))) > ^ Nitpick: phases no longer have to end in #t, though it > doesn't hurt. > >> + (propagated-inputs >> + `(("python-click" ,python-click) >> + ("python-dotenv" ,python-dotenv-0.13.0) >> + ("python-ruamel.yaml" ,python-ruamel.yaml) >> + ("python-toml" ,python-toml))) >> + (native-inputs >> + `(("make" ,gnu-make) >> + ("python-codecov" ,python-codecov) >> + ("python-configobj" ,python-configobj) >> + ("python-colorama" ,python-colorama-0.4.1) >> + ("python-django" ,python-django) >> + ("python-flake8" ,python-flake8) >> + ("python-flake8-debugger" ,python-flake8-debugger) >> + ("python-flake8-print" ,python-flake8-print) >> + ("python-flake8-todo" ,python-flake8-todo) >> + ("python-flask" ,python-flask) >> + ("python-future" ,python-future) >> + ("python-pep8-naming" ,python-pep8-naming) >> + ("python-pytest" ,python-pytest-6) >> + ("python-pytest-cov" ,python-pytest-cov) >> + ("python-pytest-forked" ,python-pytest-forked) >> + ("python-pytest-mock" ,python-pytest-mock) >> + ("python-pytest-xdist" ,python-pytest-xdist) >> + ("python-radon" ,python-radon))) > > With the test_only target, I think only a few of these are actually > required. Also, configobj should probably be a propagated input as > dynaconf uses it for ini files. I've attached a patch below. > > Notably, this seems to make python-flake8-debugger, python-flake8-todo, > python-pep8-naming and python-colorama-0.4.1 unneccessary (I think > because they are used for code linting, and the test_only target doesn't > do linting). WDYT? > > (Even if they aren't necessary for packaging dynaconf, you're still > welcome to send them as separate patches :) > >> + (home-page >> + "https://github.com/rochacbruno/dynaconf") > ^ Nitpick: this can go on one line > >> + (synopsis >> + "The dynamic configurator for your Python Project") > ^ Likewise > >> + (description >> + "This package provides @code{dynaconf} the dynamic configurator for >> +your Python Project.") > > Even as someone who has used python a lot before, this doesn't tell me > anything about what dynaconf actually does or why I might want to > install it. (Or, is it even an end-user package?) For examples, take a > look at pretty much any package which has more than two lines in its > description (like, say, python-seaborn). I know writing a good > description can be difficult, but they tend to stick around and read by > lots of people, so getting it right the first time is important! > > > > diff --git a/gnu/packages/python-xyz.scm b/gnu/packages/python-xyz.scm > index f4f3b7fb3f..58defd9fcc 100644 > --- a/gnu/packages/python-xyz.scm > +++ b/gnu/packages/python-xyz.scm > @@ -26380,28 +26380,16 @@ It implements advanced Python dictionaries with dot notation access.") > #t))))) > (propagated-inputs > `(("python-click" ,python-click) > + ("python-configobj" ,python-configobj) > ("python-dotenv" ,python-dotenv-0.13.0) > ("python-ruamel.yaml" ,python-ruamel.yaml) > ("python-toml" ,python-toml))) > (native-inputs > - `(("make" ,gnu-make) > - ("python-codecov" ,python-codecov) > - ("python-configobj" ,python-configobj) > - ("python-colorama" ,python-colorama-0.4.1) > - ("python-django" ,python-django) > - ("python-flake8" ,python-flake8) > - ("python-flake8-debugger" ,python-flake8-debugger) > - ("python-flake8-print" ,python-flake8-print) > - ("python-flake8-todo" ,python-flake8-todo) > + `(("python-django" ,python-django) > ("python-flask" ,python-flask) > - ("python-future" ,python-future) > - ("python-pep8-naming" ,python-pep8-naming) > ("python-pytest" ,python-pytest-6) > ("python-pytest-cov" ,python-pytest-cov) > - ("python-pytest-forked" ,python-pytest-forked) > - ("python-pytest-mock" ,python-pytest-mock) > - ("python-pytest-xdist" ,python-pytest-xdist) > - ("python-radon" ,python-radon))) > + ("python-pytest-mock" ,python-pytest-mock))) > (home-page > "https://github.com/rochacbruno/dynaconf") > (synopsis > > > -- > Sarah
Dear Sarah, On 8/29/21 12:46 AM, Sarah Morgensen wrote: > Did you receive my other email? I found that in fact none of the flake8 > packages, as well as some others, are actually required. I apologise if > merging these bugs caused it to get lost! I'll quote it below. No I did not :( . Also I'm sorry about the confusion with the multiple bug reports, I forgot to send the patches directly to the ticket address. > * gnu/packages/python-xyz.scm (python-colorama-0.4.1): New variable, > (python-dotenv-0.13.0): New variable, > (dynaconf): New variable. >> Packages typically get one commit per package (so this would be three >> commits). I wasn't sure about this because right now those exact versions are only needed by dynaconf. Anyway I divided them in three commits. >>> * gnu/packages/patches/dynaconf-Unvendor-dependencies.patch: New file. >> ^ an extra space slipped in here. >> >>> [...] >>> + (arguments >>> + `(#:phases >>> + (modify-phases %standard-phases >>> + (replace 'check >>> + (lambda* (#:key tests? outputs #:allow-other-keys) >>> + (when tests? >>> + (setenv "PATH" >>> + (string-append (assoc-ref outputs "out") "/bin:" >>> + (getenv "PATH"))) >>> + ;; These tests depend on hvac and a >>> + ;; live Vault process. >>> + (delete-file "tests/test_vault.py") >>> + (invoke "make" "test_only")) >>> + #t))))) >> ^ Nitpick: phases no longer have to end in #t, though it >> doesn't hurt. Thank you I didn't know it, fixed! >> >>> + (propagated-inputs >>> + `(("python-click" ,python-click) >>> + ("python-dotenv" ,python-dotenv-0.13.0) >>> + ("python-ruamel.yaml" ,python-ruamel.yaml) >>> + ("python-toml" ,python-toml))) >>> + (native-inputs >>> + `(("make" ,gnu-make) >>> + ("python-codecov" ,python-codecov) >>> + ("python-configobj" ,python-configobj) >>> + ("python-colorama" ,python-colorama-0.4.1) >>> + ("python-django" ,python-django) >>> + ("python-flake8" ,python-flake8) >>> + ("python-flake8-debugger" ,python-flake8-debugger) >>> + ("python-flake8-print" ,python-flake8-print) >>> + ("python-flake8-todo" ,python-flake8-todo) >>> + ("python-flask" ,python-flask) >>> + ("python-future" ,python-future) >>> + ("python-pep8-naming" ,python-pep8-naming) >>> + ("python-pytest" ,python-pytest-6) >>> + ("python-pytest-cov" ,python-pytest-cov) >>> + ("python-pytest-forked" ,python-pytest-forked) >>> + ("python-pytest-mock" ,python-pytest-mock) >>> + ("python-pytest-xdist" ,python-pytest-xdist) >>> + ("python-radon" ,python-radon))) >> With the test_only target, I think only a few of these are actually >> required. Also, configobj should probably be a propagated input as >> dynaconf uses it for ini files. I've attached a patch below. >> >> Notably, this seems to make python-flake8-debugger, python-flake8-todo, >> python-pep8-naming and python-colorama-0.4.1 unneccessary (I think >> because they are used for code linting, and the test_only target doesn't >> do linting). WDYT? >> >> (Even if they aren't necessary for packaging dynaconf, you're still >> welcome to send them as separate patches :) I definitely agree, I'll send them as separate patches. >> >>> + (home-page >>> + "https://github.com/rochacbruno/dynaconf") >> ^ Nitpick: this can go on one line >> >>> + (synopsis >>> + "The dynamic configurator for your Python Project") >> ^ Likewise >> >>> + (description >>> + "This package provides @code{dynaconf} the dynamic configurator for >>> +your Python Project.") >> Even as someone who has used python a lot before, this doesn't tell me >> anything about what dynaconf actually does or why I might want to >> install it. (Or, is it even an end-user package?) For examples, take a >> look at pretty much any package which has more than two lines in its >> description (like, say, python-seaborn). I know writing a good >> description can be difficult, but they tend to stick around and read by >> lots of people, so getting it right the first time is important! I updated the description with the feature set provided on the main website, it should be a little more clear now . I'm sending an updated patchset, thank you for your time :) Giacomo
Dear guixers, I'm sending a new patch set rebased on current master where I updated dynaconf to version 3.1.7. Thank you for your time, paul
On Sat, Nov 20, 2021 at 12:40:36AM +0100, paul via Guix-patches via wrote: > Dear guixers, > > I'm sending a new patch set rebased on current master where I updated > dynaconf to version 3.1.7. > > Thank you for your time, Thank you for keeping up with this patch set for so long! I ended up not applying the ruamel.yaml patch since it broke some other packages, but the other 3 were no problem. Patches pushed! Thanks.
diff --git a/gnu/packages/python-xyz.scm b/gnu/packages/python-xyz.scm index 61db9febb8..fc7f08a720 100644 --- a/gnu/packages/python-xyz.scm +++ b/gnu/packages/python-xyz.scm @@ -64,7 +64,7 @@ ;;; Copyright © 2019, 2020 Alex Griffin <a@ajgrf.com> ;;; Copyright © 2019, 2020 Pierre Langlois <pierre.langlois@gmx.com> ;;; Copyright © 2019 Jacob MacDonald <jaccarmac@gmail.com> -;;; Copyright © 2019, 2020 Giacomo Leidi <goodoldpaul@autistici.org> +;;; Copyright © 2019, 2020, 2021 Giacomo Leidi <goodoldpaul@autistici.org> ;;; Copyright © 2019 Wiktor Żelazny <wzelazny@vurv.cz> ;;; Copyright © 2019, 2020, 2021 Tanguy Le Carrour <tanguy@bioneland.org> ;;; Copyright © 2019, 2021 Mădălin Ionel Patrașcu <madalinionel.patrascu@mdc-berlin.de> @@ -9456,6 +9456,31 @@ These should be used in preference to using a backslash for line continuation. @end quotation") (license license:asl2.0))) +(define-public python-flake8-debugger + (package + (name "python-flake8-debugger") + (version "4.0.0") + (source + (origin + (method url-fetch) + (uri (pypi-uri "flake8-debugger" version)) + (sha256 + (base32 + "19pdfx0rb3k1i8hxfjdi8ndd6ayzq8g1041j8zdq256vyxvwfgg4")))) + (build-system python-build-system) + (propagated-inputs + `(("python-flake8" ,python-flake8) + ("python-pycodestyle" ,python-pycodestyle) + ("python-six" ,python-six))) + (home-page + "https://github.com/jbkahn/flake8-debugger") + (synopsis + "Ipdb/pdb statement checker plugin for flake8") + (description + "This package provides the @code{flake8-debugger} Python module, +an ipdb/pdb statement checker plugin for flake8.") + (license license:expat))) + (define-public python-flake8-implicit-str-concat (package (name "python-flake8-implicit-str-concat")