Opened 6 months ago

Closed 6 months ago

#26770 closed defect (wontfix)

Patch pynac with PR: 336

Reported by: vklein Owned by:
Priority: major Milestone: sage-duplicate/invalid/wontfix
Component: packages: standard Keywords: pynac, python3
Cc: chapoton, embray, rws Merged in:
Authors: Vincent Klein Reviewers:
Report Upstream: N/A Work issues:
Branch: u/vklein/patch_pynac_with_pr__366 (Commits) Commit: 3907c0bb31edbea6ebb2f03397d731d0f00c80c4
Dependencies: Stopgaps:

Description (last modified by chapoton)

Following #25979, this patch apply the modification from E.Bray's ​PR:336.

This fix bugs in python3 like this one : 

sage: log(int(88705158), 4)
1

Change History (14)

comment:1 Changed 6 months ago by vklein

  • Branch set to u/vklein/patch_pynac_with_pr__366

comment:2 Changed 6 months ago by vklein

  • Authors set to Vincent Klein
  • Commit set to 3907c0bb31edbea6ebb2f03397d731d0f00c80c4
  • Status changed from new to needs_review

New commits:

​3907c0bTrac #26770: Patch pynac to fix bug with long base ...

comment:3 Changed 6 months ago by vklein

  • Keywords pynac python3 added

comment:4 Changed 6 months ago by gh-timokau

Wouldn't it be better to just cut a new pynac bugfix release? Since it advertises itself as a "Sage Math support library" it doesn't make much sense that sage would have to patch it.

comment:5 Changed 6 months ago by chapoton

  • Cc rws added

comment:6 Changed 6 months ago by embray

For some reason I thought this was already fixed--indeed the solution here would be to release a new pynac.

comment:7 Changed 6 months ago by vklein

This is the best solution of course. My only question is when can we expect a new release?

comment:8 Changed 6 months ago by chapoton

  • Description modified (diff)
  • Summary changed from Patch pynac with PR: 366 to Patch pynac with PR: 336

comment:9 Changed 6 months ago by rws

Patching packages from within Sage is not a bad thing. I just hope my upcoming Pynac upgrade ticket gets more review than this one.

comment:10 follow-up: Changed 6 months ago by gh-timokau

I disagree, patching packages from within sage *is* a bad thing. It greatly increases the workload for packagers, breaks assumptions of users and may result in confusing upstream bug reports. So at least from my point of view this did get reviewed, with the verdict being "lets just do a proper update instead".

comment:11 in reply to: ↑ 10 Changed 6 months ago by embray

Replying to gh-timokau:

I disagree, patching packages from within sage *is* a bad thing.

+1 we're trying to reduce the dependence on "sage-the-distribution" which has created an inordinate maintenance burden for the entire community.

comment:12 Changed 6 months ago by vklein

  • Milestone changed from sage-8.5 to sage-duplicate/invalid/wontfix

Ok then. Wontfix seems to be the majority view for this ticket

comment:13 Changed 6 months ago by vklein

  • Status changed from needs_review to positive_review

comment:14 Changed 6 months ago by embray

  • Resolution set to wontfix
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.