Skip to content

uv migration#92

Open
itsmeadarsh2008 wants to merge 8 commits intospiraldb:developfrom
itsmeadarsh2008:develop
Open

uv migration#92
itsmeadarsh2008 wants to merge 8 commits intospiraldb:developfrom
itsmeadarsh2008:develop

Conversation

@itsmeadarsh2008
Copy link
Copy Markdown

No description provided.

@CLAassistant
Copy link
Copy Markdown

CLAassistant commented Feb 2, 2025

CLA assistant check
All committers have signed the CLA.

Copy link
Copy Markdown

@jorenham jorenham left a comment

Choose a reason for hiding this comment

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

Thanks for this! I left a few cheeky comment; feel free to ignore them anyway you see fit 👌🏻.

Although it's a bit worrying that I'm the first to review this especially because I'm just a bypasser that has never contributed before that randomly started picking nits for no reason at all

Comment on lines +24 to +25
- name: 🐍 Set up Python
uses: actions/setup-python@v5
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

is this still needed?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

🤔

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

just run uv python install

Comment thread .github/workflows/ci.yml
id: cached-poetry-dependencies
uses: actions/cache@v4
- name: 📦 Install uv
uses: astral-sh/setup-uv@v4
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

In the meantime this setup-uv action has gotten a v5 release.

Also, by explicitly setting a python-version, you avoid having to use a .python-version file

Suggested change
uses: astral-sh/setup-uv@v4
uses: astral-sh/setup-uv@v5
with:
python-version: "3.12"

Copy link
Copy Markdown
Author

@itsmeadarsh2008 itsmeadarsh2008 Apr 19, 2025

Choose a reason for hiding this comment

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

I think using a .python-version uses a standard way is a better way to declare compatible python version which I can refer to anywhere in the code, if I use it explicitly, I would have edit all the places where I referred to the incompatibility whenever I had to increment the python version.

Edit: Anyways, I am good with using v5

Comment thread .github/workflows/ci.yml Outdated
Comment thread .github/workflows/ci.yml Outdated
Comment thread .github/workflows/ci.yml
Comment on lines +23 to +24
- name: 🐍 Set up Python
uses: actions/setup-python@v5
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

There's no need for this; uv takes care of it all :)

(I can't suggest it as a change because of the interrupted diff unfortunately)

Comment thread .github/workflows/ci.yml Outdated
id: cached-poetry-dependencies
uses: actions/cache@v4
- name: 📦 Install uv
uses: astral-sh/setup-uv@v4
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

see my comment above for the juicy deets:

Suggested change
uses: astral-sh/setup-uv@v4
uses: astral-sh/setup-uv@v4
with:
python-version: "3.12"

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Comment thread .python-version
- name: Poetry Build
run: poetry build -v
- name: Publish package distributions to PyPI
- name: 🚀 Publish to PyPI
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Maybe you're already aware, but uv publish is also a thing nowadays:
https://docs.astral.sh/uv/guides/package/#publishing-your-package

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

https://docs.astral.sh/uv/guides/package/#publishing-your-package
It requires tokens to authenticate with the PyPI server whereas the pypa action doesn't need that. I would prefer using loginless method

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

hm yea... that would complicate things

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

By loginless, do you mean trusted publishing? uv publish --trusted-publishing

Comment thread pyproject.toml Outdated
itsmeadarsh2008 and others added 4 commits April 19, 2025 15:29
Co-authored-by: Joren Hammudoglu <jhammudoglu@gmail.com>
Co-authored-by: Joren Hammudoglu <jhammudoglu@gmail.com>
Co-authored-by: Joren Hammudoglu <jhammudoglu@gmail.com>
Co-authored-by: Joren Hammudoglu <jhammudoglu@gmail.com>
@jorenham
Copy link
Copy Markdown

@delta003 or @0ax1, would you mind taking a look?

- name: Poetry Build
run: poetry build -v
- name: Publish package distributions to PyPI
- name: 🚀 Publish to PyPI
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

By loginless, do you mean trusted publishing? uv publish --trusted-publishing

Comment thread .python-version Outdated
Comment thread pyproject.toml Outdated
Comment thread pyproject.toml
Comment on lines +30 to +32
"fibonacci",
"fibonacci/*.so",
"fibonacci/*.pyi",
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Suggested change
"fibonacci",
"fibonacci/*.so",
"fibonacci/*.pyi",
"fibonacci/*.so",
"fibonacci/*.pyi",

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Was there a reason you were adding fibonacci in both build targets?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

So, it generated that way.

id: cached-poetry-dependencies
uses: actions/cache@v4
- name: 📦 Install uv
uses: astral-sh/setup-uv@v4
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Comment on lines +24 to +25
- name: 🐍 Set up Python
uses: actions/setup-python@v5
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

just run uv python install

itsmeadarsh2008 and others added 2 commits April 28, 2025 16:50
Co-authored-by: Marko Bakovic <delta003@users.noreply.github.com>
Co-authored-by: Marko Bakovic <delta003@users.noreply.github.com>
Comment thread pyproject.toml
]

[tool.poetry.build]
script = "build.py"
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I missed this the first time. Does hatchling backend runs it automatically? I can't seem to find that

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

I guess so.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Have you tried installing a built wheel (uv build in this project, uv add --wheel path to the built wheel), assuming there is no prebuilt .so file? Because when I build the wheel from your branch, I get what seems to be an empty wheel:

{
  "abi": [
    "none"
  ],
  "arch": [
    "any"
  ],
  "buildver": null,
  "derived": {
    "dependencies": [],
    "description_in_body": true,
    "description_in_headers": false,
    "keyword_separator": null,
    "keywords": [],
    "modules": [
      "fibonacci"
    ],
    "readme_renders": true
  },
  "dist_info": {
    "metadata": {
      "author_email": "\"Fulcrum Inc.\" <hello@fulcrum.so>",
      "description": {
        "length": 601
      },
      "description_content_type": "text/markdown",
      "license_file": [
        "LICENSE"
      ],
      "metadata_version": "2.4",
      "name": "ziggy-pydust-template",
      "requires_python": "~=3.11",
      "version": "0.1.0"
    },
    "record": [
      {
        "digests": {
          "sha256": "47DEQpj8HBSa-_TImW-5JCeuQeRkm5NMpJWZG3hSuFU"
        },
        "path": "fibonacci/__init__.py",
        "size": 0
      },
      {
        "digests": {
          "sha256": "ZSJeVWewAuTCRB52GV1cJEXLqj41nuSOBNIDg8nM-_w"
        },
        "path": "fibonacci/_lib.pyi",
        "size": 648
      },
      {
        "digests": {
          "sha256": "PHaZgv10bSvANoRvYv38PA6D5PBbz56aBsUM-i4zNuQ"
        },
        "path": "ziggy_pydust_template-0.1.0.dist-info/METADATA",
        "size": 801
      },
      {
        "digests": {
          "sha256": "qtCwoSJWgHk21S1Kb4ihdzI2rlJ1ZKaIurTj_ngOhyQ"
        },
        "path": "ziggy_pydust_template-0.1.0.dist-info/WHEEL",
        "size": 87
      },
      {
        "digests": {
          "sha256": "xx0jnfkXJvxRnG63LTGOxlggYnIysveWIZ6H3PNdCrQ"
        },
        "path": "ziggy_pydust_template-0.1.0.dist-info/licenses/LICENSE",
        "size": 11357
      },
      {
        "digests": {},
        "path": "ziggy_pydust_template-0.1.0.dist-info/RECORD",
        "size": null
      }
    ],
    "wheel": {
      "generator": "hatchling 1.27.0",
      "root_is_purelib": true,
      "tag": [
        "py3-none-any"
      ],
      "wheel_version": "1.0"
    }
  },
  "file": {
    "digests": {
      "md5": "06b6bc059b259aa3f251523a9b7db06d",
      "sha256": "3636d1b4a09d33430a3bb2548cc31f36227e4c62ef775772f7359f0be3591ab6"
    },
    "size": 5992
  },
  "filename": "ziggy_pydust_template-0.1.0-py3-none-any.whl",
  "project": "ziggy_pydust_template",
  "pyver": [
    "py3"
  ],
  "valid": true,
  "version": "0.1.0"
}

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.

4 participants