pythonPackages.diagrams: init at 0.17.0 #101533

addict3d · 2020-10-24T06:51:06Z

Motivation for this change

Add python diagrams library. Generate nice architecture diagrams
with python code, allowing for rapid iteration.

The images are rendered using dot from graphviz. It outputs your choice of [png jpg svg pdf]

Things done

I generated some diagrams using this library, with both of the following shell expressions. There are many examples on the project's website.

nix-shell -I nixpkgs=./ -p python3 python3Packages.diagrams python3Packages.graphviz

nix-shell -I nixpkgs=./ -E 'with import <nixpkgs> {}; (python3.withPackages (ps: [ps.diagrams ps.graphviz])).env'

The closure size is ~150M.
The library itself has ~20M in images.

evanjs · 2020-10-24T06:55:43Z

Please move the maintainers.list change into a separate commit

pkgs/development/python-modules/diagrams/default.nix

evanjs · 2020-10-24T07:05:27Z

Perhaps this could be added to all-packages as well.

e.g.

nixpkgs/pkgs/top-level/all-packages.nix

Line 22650 in 91358d4

openrazer-daemon = with python3Packages; toPythonApplication openrazer-daemon;

addict3d · 2020-10-24T13:56:19Z

Perhaps this could be added to all-packages as well.

e.g.

nixpkgs/pkgs/top-level/all-packages.nix

Line 22650 in 91358d4

openrazer-daemon = with python3Packages; toPythonApplication openrazer-daemon;

Oh cool. I wasn't aware of that. Added it.

Rebased to resolve the suggestions. I've tried my best to follow the nixpkgs and python-modules conventions and am happy to learn more and make any changes.

mweinelt

That looks pretty good already.

It is customary for python packages to get the checkPhase running. Currently no tests are being found, even though upstream has tests. This is because the PyPi Package doesn't include any.

----------------------------------------------------------------------
Ran 0 tests in 0.001s

Also the default checkPhase might not work for this package. To find out how the test suite is run it is usually helpful to check how their CI is set up.

https://github.com/mingrammer/diagrams/blob/master/.github/workflows/test.yml#L35

Please try and get the tests running.

mweinelt · 2020-10-25T15:08:23Z

pkgs/development/python-modules/diagrams/default.nix

+  version = "0.17.0";
+  disabled = ! isPy3k;
+
+  # TODO consider packaging from Github: https://github.com/mingrammer/diagrams


This should be easy. Use fetchFromGitHub instead of fetchPypi and set format = "pyproject";. I assume we would then have the tests/ directory.

I spent a few hours working on this. I am far from a python expert, but it seems like the GitHub repo is structurally quite different from the pypi tarball. I will upload a new branch in a bit with my (unsuccessful) efforts.

You can see my (stalled) progress on packaging from GitHub here: #102060

@evanjs also had what is probably a terrible idea for tests, which is to 'cherry-pick' just the tests from GitHub...

addict3d · 2020-10-26T20:14:20Z

Thanks. I will take a look at it.

…

On Sun, Oct 25, 2020, 11:13 AM Martin Weinelt ***@***.***> wrote: ***@***.**** requested changes on this pull request. That looks pretty good already. It is customary for python packages to get the checkPhase running. Currently no tests are being found, even though upstream has tests. This is because the PyPi Package doesn't include any. ---------------------------------------------------------------------- Ran 0 tests in 0.001s Also the default checkPhase might not work for this package. To find out how the test suite is run it is usually helpful to check how their CI is set up. https://github.com/mingrammer/diagrams/blob/master/.github/workflows/test.yml#L35 Please try and get the tests running. ------------------------------ In pkgs/development/python-modules/diagrams/default.nix <#101533 (comment)>: > @@ -0,0 +1,68 @@ +{ lib +, fetchFromGitHub +, buildPythonPackage +, fetchPypi +, isPy3k +, graphviz +}: + +buildPythonPackage rec { + pname = "diagrams"; + version = "0.17.0"; + disabled = ! isPy3k; + + # TODO consider packaging from Github: https://github.com/mingrammer/diagrams This should be easy. Use fetchFromGitHub instead of fetchPypi and set format = "pyproject";. I assume we would then have the tests/ directory. — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#101533 (review)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAAMAS4NKIHC3VS6HL72ADLSMQ6A3ANCNFSM4S5MGCDQ> .

pkgs/top-level/all-packages.nix

addict3d · 2020-10-27T17:36:58Z

For me personally, I'd make use of this through a python environment, rather than using it from top-level. Happy to change the PR to match best practices.

…

On Tue, Oct 27, 2020, 1:16 PM Evan Stoll ***@***.***> wrote: ***@***.**** commented on this pull request. ------------------------------ In pkgs/top-level/all-packages.nix <#101533 (comment)>: > @@ -3145,6 +3145,8 @@ in dev86 = callPackage ../development/compilers/dev86 { }; + diagrams = with python3.pkgs; toPythonApplication diagrams; In the case of OpenRazer, I think it was appropriate due to the nature of the package, but I am also curious about best practices, especially in this case. — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#101533 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAAMASZF6U77QIWDNTOH2BTSM355TANCNFSM4S5MGCDQ> .

addict3d · 2020-10-29T15:23:05Z

Here is my attempt to package this from the upstream GitHub instead of the pypi tarball. Currently this produces an empty python module. I'm not (yet) familiar with python packaging to understand what is happening.

https://github.com/addict3d/nixpkgs/tree/add-python-diagrams-from-github

It appears that the library code is generated correctly and the tests all pass, but getting the library working is important.

addict3d · 2020-10-29T15:53:11Z

I removed the top-level/all-packages entry because it doesn't make sense as a binary.

You need a python environment with this library, and then to produce an image, call python my-diagram-code.py (where your code imports diagrams).

Add python diagrams library. Generate nice architecture diagrams with python code, allowing for rapid iteration. The images are rendered using dot from graphviz. Closes NixOS#101532

evanjs · 2020-10-29T15:58:45Z

Since the setup.py and tests directories don't seem to both be available from a single source, I was wondering if it's acceptable to pull in both sources.

Not quite sure what else can be done if we can't determine how the GitHub sources can be installed or etc.
Anyway, the following seems to work, but feels awfully hacky to me.

I also wonder if sourcing GitHub and pulling the setup.py from Pypi would be any better 🤷‍♂️

sources from Pypi with tests from GitHub

diff --git a/pkgs/development/python-modules/diagrams/default.nix b/pkgs/development/python-modules/diagrams/default.nix
index 1f05abab32c..bbfa50a5a3c 100644
--- a/pkgs/development/python-modules/diagrams/default.nix
+++ b/pkgs/development/python-modules/diagrams/default.nix
@@ -4,17 +4,28 @@
 , fetchPypi
 , isPy3k
 , graphviz
+, pytestCheckHook
 }:

-buildPythonPackage rec {
+let
   pname = "diagrams";
   version = "0.17.0";
+  gitHubSrc = fetchFromGitHub {
+    owner = "mingrammer";
+    repo = pname;
+    rev = "v${version}";
+    sha256 = "1h5i9xb4fsmy81sznvqycxl712d9k695vznkpx2wm1ki7hvz86s8";
+  };
+in
+buildPythonPackage rec {
   disabled = ! isPy3k;
+  inherit pname version;

   # TODO consider packaging from Github: https://github.com/mingrammer/diagrams
   # note: outscale package is broken, __init__.py is missing
   src = fetchPypi {
     inherit pname version;
+
     sha256 = "17v5nzswva74bawxrvkza8a5d9aqyhb2gkncp3fqv6rszmk62hw6";
   };

@@ -22,6 +33,8 @@ buildPythonPackage rec {
     ./diags_fix_aliases.patch
   ];

+  checkInputs = [ pytestCheckHook ];
+
   # This script is patching setup.py from the pypi tarball, to retain
   # the png images which this library uses to generate diagrams.
   postPatch = ''
@@ -55,6 +68,8 @@ buildPythonPackage rec {
       done
       echo "    ],)"
     } >> setup.py
+
+    cp -r ${gitHubSrc}/tests .
   '';

   buildInputs = [ graphviz ];

mweinelt · 2020-10-29T17:34:04Z

I thought we could fetch this from GitHub and set format = "pyproject"; so it would build from the pyproject.toml. We don't strictly need a setup.py. But IIRC the following part is missing in pyproject.toml.

[build-system]
requires = ["poetry-core>=1.0.0"]
build-backend = "poetry.core.masonry.api"

The author only uses this with poetry to develop, build and publish, so they don't need that explicitly.

The test phase uses the unittest module: https://github.com/mingrammer/diagrams/blob/master/.github/workflows/test.yml#L35

The problems remind me of supakeen/pinnwand#93.

mweinelt · 2020-10-29T17:46:23Z

Builds after patching the upstream git checkout like this and adding format = "pyproject";:

diff --git a/pyproject.toml b/pyproject.toml
index 8725f84..c134ab8 100644
--- a/pyproject.toml
+++ b/pyproject.toml
@@ -11,8 +11,8 @@ include = ["resources/**/*"]
 
 [tool.poetry.dependencies]
 python = "^3.6"
-graphviz = "^0.13.2"
-jinja2 = "^2.10"
+graphviz = "^0.14.1"
+jinja2 = "^2.11"
 contextvars = { version = "^2.4", python = "~3.6" }
 
 [tool.poetry.dev-dependencies]
@@ -24,3 +24,7 @@ isort = "^4.3"
 
 [tool.black]
 line-length = 120
+
+[build-system]
+requires = ["poetry_core>=1.0.0"]
+build-backend = "poetry.core.masonry.api"

You're also missing jinja2 and both graphviz and jinja2 should be propagatedBuildInputs if you're planning to use this as a library. See https://github.com/mingrammer/diagrams/blob/v0.17.0/pyproject.toml#L12-L16

You'll also need poetry in your nativeBuildInputs.

martinbaillie · 2021-01-02T09:47:29Z

hey @addict3d are you still planning on taking this package into Nixpkgs? I've been using your branch for a while and would be keen to see it merged

pkgs/development/python-modules/diagrams/default.nix

addict3d · 2021-03-21T15:52:01Z

hey @addict3d are you still planning on taking this package into Nixpkgs? I've been using your branch for a while and would be keen to see it merged

oh. err sorry about that. yes I will get back to this and address the comments

addict3d · 2021-04-10T16:00:12Z

I'm closing this in favor of packaging directly from upstream (GitHub) rather than pypi. This way the package will include tests, and it removes one layer of indirection.

See #102060

addict3d requested review from FRidh and jonringer as code owners October 24, 2020 06:51

ofborg bot added the 6.topic: python label Oct 24, 2020

ofborg bot added 8.has: package (new) 11.by: package-maintainer 10.rebuild-darwin: 1-10 10.rebuild-linux: 1-10 labels Oct 24, 2020

evanjs reviewed Oct 24, 2020

View reviewed changes

pkgs/development/python-modules/diagrams/default.nix Outdated Show resolved Hide resolved

maintainers: add addict3d

1827b73

addict3d force-pushed the add-python-diagrams-from-pypi branch from 0ee823b to 847aa7e Compare October 24, 2020 13:52

mweinelt requested changes Oct 25, 2020

View reviewed changes

sikmir reviewed Oct 27, 2020

View reviewed changes

pkgs/top-level/all-packages.nix Outdated Show resolved Hide resolved

addict3d mentioned this pull request Oct 29, 2020

Add python diagrams from GitHub #102060

Merged

10 tasks

addict3d force-pushed the add-python-diagrams-from-pypi branch from 847aa7e to 5857fa9 Compare October 29, 2020 15:51

pythonPackages.diagrams: init at 0.17.0

0fcd1f6

Add python diagrams library. Generate nice architecture diagrams with python code, allowing for rapid iteration. The images are rendered using dot from graphviz. Closes NixOS#101532

addict3d force-pushed the add-python-diagrams-from-pypi branch from 5857fa9 to 0fcd1f6 Compare October 29, 2020 15:56

SuperSandro2000 requested changes Mar 21, 2021

View reviewed changes

pkgs/development/python-modules/diagrams/default.nix Show resolved Hide resolved

pkgs/development/python-modules/diagrams/default.nix Show resolved Hide resolved

addict3d closed this Apr 10, 2021

addict3d deleted the add-python-diagrams-from-pypi branch April 12, 2021 17:59

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

pythonPackages.diagrams: init at 0.17.0 #101533

pythonPackages.diagrams: init at 0.17.0 #101533

addict3d commented Oct 24, 2020

evanjs commented Oct 24, 2020

evanjs commented Oct 24, 2020

addict3d commented Oct 24, 2020

mweinelt left a comment

mweinelt Oct 25, 2020

addict3d Oct 28, 2020

addict3d Oct 29, 2020

addict3d commented Oct 26, 2020 via email

addict3d commented Oct 27, 2020 via email

addict3d commented Oct 29, 2020

addict3d commented Oct 29, 2020

evanjs commented Oct 29, 2020

mweinelt commented Oct 29, 2020 •

edited

mweinelt commented Oct 29, 2020 •

edited

martinbaillie commented Jan 2, 2021

addict3d commented Mar 21, 2021

addict3d commented Apr 10, 2021

pythonPackages.diagrams: init at 0.17.0 #101533

pythonPackages.diagrams: init at 0.17.0 #101533

Conversation

addict3d commented Oct 24, 2020

Motivation for this change

Things done

evanjs commented Oct 24, 2020

evanjs commented Oct 24, 2020

addict3d commented Oct 24, 2020

mweinelt left a comment

Choose a reason for hiding this comment

mweinelt Oct 25, 2020

Choose a reason for hiding this comment

addict3d Oct 28, 2020

Choose a reason for hiding this comment

addict3d Oct 29, 2020

Choose a reason for hiding this comment

addict3d commented Oct 26, 2020 via email

addict3d commented Oct 27, 2020 via email

addict3d commented Oct 29, 2020

addict3d commented Oct 29, 2020

evanjs commented Oct 29, 2020

mweinelt commented Oct 29, 2020 • edited

mweinelt commented Oct 29, 2020 • edited

martinbaillie commented Jan 2, 2021

addict3d commented Mar 21, 2021

addict3d commented Apr 10, 2021

mweinelt commented Oct 29, 2020 •

edited

mweinelt commented Oct 29, 2020 •

edited