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

Enables python 3.13 #871

Open
wants to merge 14 commits into
base: master
Choose a base branch
from

Conversation

rudolfix
Copy link
Contributor

@rudolfix rudolfix commented Jan 8, 2025

This PR makes pendulum work on Python 3.13. We are using it a lot in our library: https://github.com/dlt-hub/dlt and initially considered forking pendulum to make it work on Python 3.13. Maybe there's a chance to merge this into main project?

I'm sure some of the changes are going too far. We are open to revert them if there's an interest from maintainers to merge this PR :)

We are running this PR with our own tests on all basic architectures and we do not see any problems

  • O3 is bumped again to make it build on all Windows architectures
  • More architectures are added to release build
  • TimeMachine is used only in tests and present only in test deps.

happy to revert/change

  • part doing the actual release commented out in github actions
  • codespeed disabled (could not make it work on our bracnch)

Copy link
Member

@ashb ashb left a comment

Choose a reason for hiding this comment

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

Looks like something equivalent was merged in #863?

Comment on lines +56 to +61
uses: snok/install-poetry@v1
with:
virtualenvs-create: true
virtualenvs-in-project: true
installer-parallel: true
version: 1.8.5
Copy link
Member

Choose a reason for hiding this comment

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

Is this change needed? I'm sure it's okay, but it does mean one extra step in the CI supply chain to worry about, so if it's not 100% strictly required, given it's not an official poetry action I'd favour sticking with just "official" curl approach?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Since we are using github default runners, simple pipx install poetry=={version} is all we need.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No problem to revert back to curl. Should I upgrade poetry to 2.0 for the whole project? lock file is not compatible. (that's why I used the action - I could pick the right version easily)

Copy link
Member

Choose a reason for hiding this comment

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

For ease of review I'd say keep the poetry upgrade/changes to a separate PR please

@@ -1,6 +1,6 @@
[project]
name = "pendulum"
version = "3.0.0"
version = "3.0.1"
Copy link
Member

Choose a reason for hiding this comment

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

FYI: Please don't change the version in the PR like this, a sepearte release PR will make this change

Copy link
Contributor Author

Choose a reason for hiding this comment

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

sure!

@rudolfix
Copy link
Contributor Author

Looks like something equivalent was merged in #863?

this is a continuation of #863. We had to upgrade O3 version again because we could not build Windows wheels. This again required us to handle some deprecations. I think @Secrus was able to merge the relevant commit.

We'll come back with more fixes. We will be testing updated wheels via our private PyPI and then update this PR.

@Secrus
Copy link
Collaborator

Secrus commented Jan 18, 2025

We have merged some updates to Poetry and CI setup. @rudolfix could you rebase the changes?

@rudolfix
Copy link
Contributor Author

@Secrus pendulum is segfaulting on alpine linux (musllinux). it is due to mimalloc being used (and not disabled). do you have anything against removing mimalloc from the rust extensions? I'm not sure what are the expected speedups from using it.

also we added any wheel for platforms that we do not have wheels with rust built. this will disable speedups automatically. I hope that is OK. PR will come soon

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.

3 participants