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

use pyproject.toml configuration #203

Merged
merged 6 commits into from
Jul 23, 2024

Conversation

mbalatsko
Copy link
Contributor

This PR unifies all configurations to be set in pyproject.toml:

  • Remove setup.py, setup.cfg, .coveragerc
  • Regenerate requirements using pip-compile from pyproject.toml
  • Remove unnecessary CLI params, that will be parsed from config in scripts and pipeline

Resolves #201

@mbalatsko mbalatsko changed the title use pyproject.toml configuration [WIP] use pyproject.toml configuration Jul 12, 2024
@mbalatsko mbalatsko changed the title [WIP] use pyproject.toml configuration use pyproject.toml configuration Jul 12, 2024
Copy link

codecov bot commented Jul 12, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 98.91%. Comparing base (c194757) to head (dee990f).

Additional details and impacted files
@@            Coverage Diff             @@
##           master     #203      +/-   ##
==========================================
- Coverage   99.13%   98.91%   -0.23%     
==========================================
  Files           1        1              
  Lines         465      462       -3     
==========================================
- Hits          461      457       -4     
- Misses          4        5       +1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@mbalatsko
Copy link
Contributor Author

@raimon49 should be ready for review

pyproject.toml Outdated Show resolved Hide resolved
setup.py Show resolved Hide resolved
@mbalatsko mbalatsko requested a review from raimon49 July 16, 2024 18:56
pyproject.toml Outdated

[tool.setuptools.dynamic]
version = {attr = "piplicenses.__version__"}
readme = {file = ["README.md", "CHANGELOG.md"]}
Copy link
Owner

Choose a reason for hiding this comment

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

@mbalatsko Thanks for finding a way to combine the README documents.

I tried uploading to TestPyPI and it returned an error regarding file content type.

ERROR    HTTPError: 400 Bad Request from https://test.pypi.org/legacy/
         The description failed to render for 'text/x-rst'. See https://test.pypi.org/help/#description-content-type for
         more information.

Don't we need a content-type key along with the file key?

readme = {file = ["README.md", "CHANGELOG.md"], content-type = "text/markdown"}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, in case we see that error. I was under intention that it is default, let me push fix for it now

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@raimon49 pushed, could you try it out please?

Copy link
Owner

Choose a reason for hiding this comment

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

Good, error resolved 👍
https://test.pypi.org/project/pip-licenses/4.6.0rc1/

One thing I saw and noticed is that the license metadata is very long full text when packaged with license = {file = “LICENSE”}.

Wouldn't the pip-licenses command create a very large table in the output? Of course, Classifiers take precedence by default, so it is not worth worrying about.

Did you change from text to file because file is the preferred method?
b05111e

Copy link
Contributor Author

Choose a reason for hiding this comment

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

According to https://packaging.python.org/en/latest/guides/writing-pyproject-toml/#license both options are equally right. I just wanted to make use of file we have in repository, no other reason.
I tried to run pip-licenses --with-system --from meta:

 pip-licenses        4.5.1            MIT License                                                                            
                                                                                                                             
                                              Copyright (c) 2018 raimon                                                      
                                                                                                                             
                                              Permission is hereby granted, free of charge, to any person obtaining a copy   
                                              of this software and associated documentation files (the "Software"), to deal  
                                              in the Software without restriction, including without limitation the rights   
                                              to use, copy, modify, merge, publish, distribute, sublicense, and/or sell      
                                              copies of the Software, and to permit persons to whom the Software is          
                                              furnished to do so, subject to the following conditions:                       
                                                                                                                             
                                              The above copyright notice and this permission notice shall be included in all 
                                              copies or substantial portions of the Software.                                
                                                                                                                             
                                              THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR     
                                              IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,       
                                              FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE    
                                              AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER         
                                              LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,  
                                              OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE  
                                              SOFTWARE.     

And it is indeed long, but if you use mixed pip-licenses --with-system --mixed:

 pip-licenses        4.5.1            MIT License 

Copy link
Contributor Author

Choose a reason for hiding this comment

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

If you'd prefer to use textual format, I have no problems fixing it in the PR. But I suggest using MIT License instead of MIT as it is more semantically correct

Copy link
Owner

Choose a reason for hiding this comment

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

Thanks for the info. It still looks like I should adopt text.

The notation MIT also seems to be a short identifier standardized by SPDX.
https://spdx.org/licenses/

However, your suggestion is also acceptable.

Can you change the text to either full or short and push it? I will merge it and ship as a new release.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Reverted, should be ready to merge now

Copy link
Owner

@raimon49 raimon49 left a comment

Choose a reason for hiding this comment

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

I made sure I was ready to merge.

Ships as a production release over the weekend.

@raimon49 raimon49 merged commit 63eb778 into raimon49:master Jul 23, 2024
6 of 7 checks passed
@raimon49
Copy link
Owner

@mbalatsko pip-licenses 5.0.0 was shipped as our work product!!
https://pypi.org/project/pip-licenses/5.0.0/

Package metadata of this project is now very modern. Thanks for your contribution 🎉

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.

Use pyproject.toml instead of setup.cfg and setup.py
3 participants