sagemath / cypari2

Python interface to the number theory library PARI/GP. Source repository for https://pypi.org/project/cypari2/
https://cypari2.readthedocs.io/en/latest/
GNU General Public License v2.0
30 stars 28 forks source link

Use cysignals hook #117

Closed kliem closed 1 year ago

kliem commented 2 years ago

@mkoeppe @videlec

This branch inserts the hooks as discussed in #109. What do you think?

It's based upon https://github.com/sagemath/cysignals/pull/166.

kliem commented 2 years ago

According to https://github.com/kliem/cypari2/runs/6660982099?check_suite_focus=true this approach works at least for the test suite (usually there are quite a lot time outs, as the current github workflow does not build cysignals linked to pari).

videlec commented 2 years ago

Great! Thanks for working on it. It will simplify a lot the distribution of cysignals.

mkoeppe commented 2 years ago

Looking great!

videlec commented 2 years ago

Why did you bump the version here? Just for testing purposes? I do not see why a version bump would be inside a merge request.

kliem commented 2 years ago

I don't know how this usually works. I just think it's irritating when the version number does not reflect the current status. Anyway the version bump is mostly needed for testing purposes, so that I can make https://github.com/kliem/cypari2/releases/tag/v2.2.0a0 and use it on https://trac.sagemath.org/ticket/33878, which in return uses it for the test in https://github.com/sagemath/cysignals/pull/166.

kliem commented 2 years ago

This pull request isn't ready yet anyway, as it depends on a cysignals version, which isn't even released yet.

dimpase commented 1 year ago

What's needed for this to make it? Is sagemath/cysignals ready for a new release?

dimpase commented 1 year ago

It would be nice to have a new release here soon, to let Pari 2.15 upgrade into Sage

dimpase commented 1 year ago

I don't see how VERSION is needed anywhere, apart from new releases. After all, our GH Actions checks are triggered by new pushes/PRs, not by version changes.

kliem commented 1 year ago

I just think merging a pull request and leaving the VERSION at an actual released version could be confusing.

On Mon, Oct 31, 2022, 17:16 Dima Pasechnik @.***> wrote:

I don't see how VERSION is needed anywhere, apart from new releases. After all, our GH Actions checks are triggered by new pushes/PRs, not by version changes.

— Reply to this email directly, view it on GitHub https://github.com/sagemath/cypari2/pull/117#issuecomment-1297335717, or unsubscribe https://github.com/notifications/unsubscribe-auth/AETJVLUJGGKWOO6MRCPULH3WF7WF3ANCNFSM5XLSP2HA . You are receiving this because you authored the thread.Message ID: @.***>

mkoeppe commented 1 year ago

Some projects solve this problem by making a commit that changes the version to some ".dev0" version right after a release

dimpase commented 1 year ago

bumping up VERSION with every merged PR isn't feasible, IMHO.

dimpase commented 1 year ago

OK, I've resolved this merge to be as supplied by the PR