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

modernize packaging #137

Merged
merged 15 commits into from
Sep 7, 2023
Merged

modernize packaging #137

merged 15 commits into from
Sep 7, 2023

Conversation

ocefpaf
Copy link
Contributor

@ocefpaf ocefpaf commented Jun 19, 2023

Use setuptools_scm instead of versioneer and pyproject.toml instead of setup.*.

Only review this one after #136 is sorted out.

@ocefpaf ocefpaf marked this pull request as draft June 19, 2023 17:32
@ocefpaf ocefpaf force-pushed the modernize_packging branch 2 times, most recently from 7a76801 to ba6b6b4 Compare June 19, 2023 20:02
@ocefpaf ocefpaf marked this pull request as ready for review June 20, 2023 18:34
@ocefpaf ocefpaf force-pushed the modernize_packging branch from 0764309 to b227394 Compare June 21, 2023 11:26
Copy link
Contributor Author

@ocefpaf ocefpaf left a comment

Choose a reason for hiding this comment

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

@ajdawson let me know if you need help navigating this PR. Sorry I ended up bloating it but this will get the CIs back online/

@@ -1,33 +0,0 @@
"""Test coding standards compliance."""
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@ajdawson I replaced this test with pre-commits. We can add more checks there if you want but I would do that in another PR to avoid bloating this one more than it already is.

Copy link
Owner

Choose a reason for hiding this comment

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

This is fine I think

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

[tool.setuptools_scm]
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Using setuptools_scm instead of versioneer b/c that one is better maintained and "blessed" by the Python packaging community.

@@ -0,0 +1,75 @@
[build-system]
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This replaces both setup.py and setup.cfg with "modern" python packaging practices.

runs-on: ${{ matrix.os }}
strategy:
matrix:
python-version: ["3.8", "3.9", "3.10"]
Copy link
Contributor Author

Choose a reason for hiding this comment

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

To support all the extras on Windows and Python 3.11 we would need to drop cdms2.
xref.: conda-forge/cdms2-feedstock#80 (comment) and conda-forge/cdtime-feedstock#52 (comment)

Copy link
Owner

Choose a reason for hiding this comment

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

Maybe the time is coming to remove the cdms2 interface in this module...

Copy link
Owner

Choose a reason for hiding this comment

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

But we can leave it like this for now and deal with that separately.

@ocefpaf
Copy link
Contributor Author

ocefpaf commented Jun 21, 2023

Twine check is failing with:

Checking eofs-0.1.dev1+g23a45fe-py3-none-any.whl: FAILED
ERROR    `long_description` has syntax errors in markup and would not be        
         rendered on PyPI.                                                      
         No content rendered from RST source.                                   
WARNING  `long_description_content_type` missing. defaulting to `text/x-rst`.   
Checking eofs-0.1.dev1+g23a45fe.tar.gz: PASSED with warnings
WARNING  `long_description_content_type` missing. defaulting to `text/x-rst`.   
WARNING  `long_description` missing.                                            
Error: Process completed with exit code 1.

I can reproduce that locally and I see that it still packages the deleted setup.cfg, no idea why/how that id happening :-/


Edit: I missed an entry for the dynamic README and it should be fixed in 30cc5c8.

@ajdawson
Copy link
Owner

Ok, so again I won't pretend that I understand every part of this, but I think this is going in the right direction so I will be happy to merge it. Thanks for the hard work (and it does look like quite a lot of work!). I'm not sure about the pypi secrets, you'll have to fill me in on what needs to be done there.

@ocefpaf
Copy link
Contributor Author

ocefpaf commented Jun 21, 2023

I'm not sure about the pypi secrets, you'll have to fill me in on what needs to be done there.

You will need to:

  1. In your PyPI account settings, scroll down to the "add API token" button

Screenshot from 2023-06-21 13-06-37

I usually name it <pkgname>__token__ and choose the scope of the package only.

  1. In you GitHub settings for eofs go to Secrets and VariablesActions → and create a Repository secret named PYPI_PASSWORD with the token value you got from 1.

Screenshot from 2023-06-21 13-10-07

PS: There is also the Trusted Publisher route but I did not try that one yet.

@ajdawson ajdawson merged commit a8ac39b into ajdawson:main Sep 7, 2023
@ajdawson
Copy link
Owner

ajdawson commented Sep 7, 2023

Sorry that took so long @ocefpaf, was locked out of PyPI account when I went to resolve this back in June, and then just forgot about it. I think this should all be set up properly now. Thanks for the effort!

@ocefpaf ocefpaf deleted the modernize_packging branch September 7, 2023 12:56
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