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

update packaging metadata #35

Merged
merged 2 commits into from
Jul 2, 2024
Merged

update packaging metadata #35

merged 2 commits into from
Jul 2, 2024

Conversation

bollwyvl
Copy link
Contributor

Thanks for this tool!

This PR updates setup.py to:

  • more formally specify the python it requires via python_requires
  • ensure the LICENSE.md is included in the distributions (not present in at least 0.2.0.tar.gz) via include_package_data
    • setuptools (or whoever's responsible) knows about LICENSE(.txt), but not .md

@codecov
Copy link

codecov bot commented Feb 24, 2021

Codecov Report

Merging #35 (0e0b539) into master (51d3c66) will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master      #35   +/-   ##
=======================================
  Coverage   76.94%   76.94%           
=======================================
  Files          23       23           
  Lines         590      590           
=======================================
  Hits          454      454           
  Misses        136      136           

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 51d3c66...0e0b539. Read the comment docs.

@hbmartin
Copy link
Owner

Thanks for the PR!
Might it be better to either convert the license to txt or explicitly in include that file (instead of all source root files)?
Also feel free to add yourself to authors section in readme :)

@bollwyvl
Copy link
Contributor Author

convert the license to txt

I wouldn't change it now... it's kinda cool that it's from the first commit!

explicitly in include that file

that's what the MANIFEST.in does... but needs the setup.py flag. Yeah, i don't understand either. As mentioned, The current sdist doesn't include it... but does include the one test .py file.

all source root file

It doesn't do that. Here's what comes out of this PR's sdist (still has that test):

graphviz2drawio-0.2.0/
graphviz2drawio-0.2.0/LICENSE.md
graphviz2drawio-0.2.0/MANIFEST.in
graphviz2drawio-0.2.0/PKG-INFO
graphviz2drawio-0.2.0/README.md
graphviz2drawio-0.2.0/graphviz2drawio/
graphviz2drawio-0.2.0/graphviz2drawio/__init__.py
graphviz2drawio-0.2.0/graphviz2drawio/__main__.py
graphviz2drawio-0.2.0/graphviz2drawio/graphviz2drawio.py
graphviz2drawio-0.2.0/graphviz2drawio/models/
graphviz2drawio-0.2.0/graphviz2drawio/models/Arguments.py
graphviz2drawio-0.2.0/graphviz2drawio/models/CoordsTranslate.py
graphviz2drawio-0.2.0/graphviz2drawio/models/DotAttr.py
graphviz2drawio-0.2.0/graphviz2drawio/models/Rect.py
graphviz2drawio-0.2.0/graphviz2drawio/models/SVG.py
graphviz2drawio-0.2.0/graphviz2drawio/models/SvgParser.py
graphviz2drawio-0.2.0/graphviz2drawio/models/__init__.py
graphviz2drawio-0.2.0/graphviz2drawio/mx/
graphviz2drawio-0.2.0/graphviz2drawio/mx/Curve.py
graphviz2drawio-0.2.0/graphviz2drawio/mx/CurveFactory.py
graphviz2drawio-0.2.0/graphviz2drawio/mx/Edge.py
graphviz2drawio-0.2.0/graphviz2drawio/mx/EdgeFactory.py
graphviz2drawio-0.2.0/graphviz2drawio/mx/GraphObj.py
graphviz2drawio-0.2.0/graphviz2drawio/mx/LinearRegression.py
graphviz2drawio-0.2.0/graphviz2drawio/mx/MxConst.py
graphviz2drawio-0.2.0/graphviz2drawio/mx/MxGraph.py
graphviz2drawio-0.2.0/graphviz2drawio/mx/Node.py
graphviz2drawio-0.2.0/graphviz2drawio/mx/NodeFactory.py
graphviz2drawio-0.2.0/graphviz2drawio/mx/Shape.py
graphviz2drawio-0.2.0/graphviz2drawio/mx/Styles.py
graphviz2drawio-0.2.0/graphviz2drawio/mx/Text.py
graphviz2drawio-0.2.0/graphviz2drawio/mx/__init__.py
graphviz2drawio-0.2.0/graphviz2drawio/version.py
graphviz2drawio-0.2.0/graphviz2drawio.egg-info/
graphviz2drawio-0.2.0/graphviz2drawio.egg-info/PKG-INFO
graphviz2drawio-0.2.0/graphviz2drawio.egg-info/SOURCES.txt
graphviz2drawio-0.2.0/graphviz2drawio.egg-info/dependency_links.txt
graphviz2drawio-0.2.0/graphviz2drawio.egg-info/entry_points.txt
graphviz2drawio-0.2.0/graphviz2drawio.egg-info/requires.txt
graphviz2drawio-0.2.0/graphviz2drawio.egg-info/top_level.txt
graphviz2drawio-0.2.0/setup.cfg
graphviz2drawio-0.2.0/setup.py
graphviz2drawio-0.2.0/test/
graphviz2drawio-0.2.0/test/test_graphs.py

and the whl

  inflating: graphviz2drawio/__init__.py  
  inflating: graphviz2drawio/__main__.py  
  inflating: graphviz2drawio/graphviz2drawio.py  
  inflating: graphviz2drawio/version.py  
  inflating: graphviz2drawio/models/Arguments.py  
  inflating: graphviz2drawio/models/CoordsTranslate.py  
  inflating: graphviz2drawio/models/DotAttr.py  
  inflating: graphviz2drawio/models/Rect.py  
  inflating: graphviz2drawio/models/SVG.py  
  inflating: graphviz2drawio/models/SvgParser.py  
  inflating: graphviz2drawio/models/__init__.py  
  inflating: graphviz2drawio/mx/Curve.py  
  inflating: graphviz2drawio/mx/CurveFactory.py  
  inflating: graphviz2drawio/mx/Edge.py  
  inflating: graphviz2drawio/mx/EdgeFactory.py  
  inflating: graphviz2drawio/mx/GraphObj.py  
  inflating: graphviz2drawio/mx/LinearRegression.py  
  inflating: graphviz2drawio/mx/MxConst.py  
  inflating: graphviz2drawio/mx/MxGraph.py  
  inflating: graphviz2drawio/mx/Node.py  
  inflating: graphviz2drawio/mx/NodeFactory.py  
  inflating: graphviz2drawio/mx/Shape.py  
  inflating: graphviz2drawio/mx/Styles.py  
  inflating: graphviz2drawio/mx/Text.py  
  inflating: graphviz2drawio/mx/__init__.py  
  inflating: graphviz2drawio-0.2.0.dist-info/LICENSE.md  
  inflating: graphviz2drawio-0.2.0.dist-info/METADATA  
  inflating: graphviz2drawio-0.2.0.dist-info/WHEEL  
  inflating: graphviz2drawio-0.2.0.dist-info/entry_points.txt  
  inflating: graphviz2drawio-0.2.0.dist-info/top_level.txt  
  inflating: graphviz2drawio-0.2.0.dist-info/RECORD  

Also feel free to add yourself to authors section in readme :)

If we can get away with it being one commit to one file, no need ❤️ !

@bollwyvl
Copy link
Contributor Author

Also, as a heads-up: since we recently got the whole graphviz build chain squared away on conda-forge (even on windows and apple m1!), i've started the process to get this package added, too (far simpler, by comparison!):

conda-forge/staged-recipes#14082

It's always great to have upstream maintainers on the team, but I'm more than happy to do the maintenance (usually just pressing the big green button). i'm presently pulling the license and the rest of the test artifacts from the github tarball, so this issue isn't blocking over there.

@bollwyvl
Copy link
Contributor Author

It's up on conda-forge 🎉, installable via:

conda install -c conda-forge graphviz2drawio

@hbmartin
Copy link
Owner

hbmartin commented Jul 2, 2024

Thank you! 🥳
Exciting to hear about Conda - I'm reviving this project so please let me know how I might be able to support that.

@hbmartin hbmartin merged commit da2f849 into hbmartin:master Jul 2, 2024
@bollwyvl bollwyvl deleted the patch-1 branch July 2, 2024 00:42
@bollwyvl
Copy link
Contributor Author

bollwyvl commented Jul 2, 2024

Sounds good.

As a repackager, there's not a lot to do to, specifically, to improve matters: mostly just following prevailing conventions. Including test (with any assets) in the sdist is helpful, but usually not productive for the .whl.

I would recommend getting off setup.py at the earliest convenience: setuptools/pip will probably just stop working with it on some upcoming major release, so getting to a pyproject.toml-based release will likely save a number of end user headaches.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants