Opened 3 years ago

Closed 3 years ago

#21623 closed defect (fixed)

Upgrade to pynac-0.6.91

Reported by: rws Owned by:
Priority: major Milestone: sage-7.4
Component: packages: standard Keywords:
Cc: paulmasson, fbissey Merged in:
Authors: Ralf Stephan Reviewers: Paul Masson, Travis Scrimshaw
Report Upstream: N/A Work issues:
Branch: 48dac6b (Commits) Commit: 48dac6b87054f77a23936b994c5163f30e381bfa
Dependencies: Stopgaps:

Description (last modified by rws)

  • compiles with clang (fix by François Bissey)
  • fix cot(float) bug; (#21365, fix by Paul Masson)
  • NaN constant; return NaN with atan2(0,0) (#21614)
  • numeric log fixes (#18970)
  • prevent some overflow exceptions in cot/csc
  • two-argument log (log to base) (#18970)
  • Support gegenbauer(n,a,x) with non-numeric a (recurrence by Carlos R. Mafra)
  • update AUTHORS

https://github.com/pynac/pynac/releases/download/pynac-0.6.91/pynac-0.6.91.tar.bz2

Change History (20)

comment:1 Changed 3 years ago by rws

  • Description modified (diff)

comment:2 Changed 3 years ago by rws

  • Branch set to u/rws/upgrade_to_pynac_0_6_91

comment:3 Changed 3 years ago by rws

  • Authors set to Ralf Stephan
  • Commit set to aeb81e3d8d5fe1459265ba1b0c3c8f6efa830770
  • Description modified (diff)
  • Status changed from new to needs_review

New commits:

b27d68dversion/chksum
aeb81e3changes to support pynac-0.6.91

comment:4 Changed 3 years ago by rws

  • Cc paulmasson fbissey added

comment:5 follow-up: Changed 3 years ago by paulmasson

  • Reviewers set to Paul Masson
  • Status changed from needs_review to needs_work

Documentation builds, but failing doctest in fast_callable.pyx.

Would like more information on "overflow exceptions in cot/csc", which is presumably this commit. What does this fix exactly?

The versioning is nonstandard. If this is a minor update it should be 0.6.9.1, otherwise 0.6.10.

comment:6 Changed 3 years ago by git

  • Commit changed from aeb81e3d8d5fe1459265ba1b0c3c8f6efa830770 to 9634b8a0b3abd225025e4ee6341f58e1114ae531

Branch pushed to git repo; I updated commit sha1. New commits:

9634b8a21623: revert a doctest change

comment:7 in reply to: ↑ 5 ; follow-up: Changed 3 years ago by rws

Replying to paulmasson:

Documentation builds, but failing doctest in fast_callable.pyx.

Curious. The doctest change seemeed necessary, but not anymore.

Would like more information on "overflow exceptions in cot/csc", which is presumably this commit. What does this fix exactly?

I think I explained this already. The only exact numeric value that can go into inverse() is zero. All others are inexact. Otherwise the doctest that fails is:

File "src/sage/functions/trig.py", line 268, in sage.functions.trig.Function_cot.__init__
Failed example:
    cot(float(0))
Exception raised:
    Traceback (most recent call last):
      File "/home/ralf/sage/local/lib/python2.7/site-packages/sage/doctest/forker.py", line 498, in _run
        self.compile_and_execute(example, compiler, test.globs)
      File "/home/ralf/sage/local/lib/python2.7/site-packages/sage/doctest/forker.py", line 861, in compile_and_execute
        exec(compiled, globs)
      File "<doctest sage.functions.trig.Function_cot.__init__[16]>", line 1, in <module>
        cot(float(Integer(0)))
      File "sage/symbolic/function.pyx", line 996, in sage.symbolic.function.BuiltinFunction.__call__ (build/cythonized/sage/symbolic/function.cpp:11008)
        res = super(BuiltinFunction, self).__call__(
      File "sage/symbolic/function.pyx", line 488, in sage.symbolic.function.Function.__call__ (build/cythonized/sage/symbolic/function.cpp:6241)
        res = g_function_eval1(self._serial,
    OverflowError: numeric::inverse(): division by zero

The versioning is nonstandard. If this is a minor update it should be 0.6.9.1, otherwise 0.6.10.

Where do you get the idea of a standard for versioning? I could use ISO 8601 of the release date as well for the version string. There is a tradition to use major/minor/micro for Pynac, and I changed micro from 9 to 91 (ninety one) to stay lexicographically consecutive.


New commits:

9634b8a21623: revert a doctest change

comment:8 Changed 3 years ago by rws

  • Status changed from needs_work to needs_review

comment:9 in reply to: ↑ 7 Changed 3 years ago by paulmasson

Replying to rws:

The versioning is nonstandard. If this is a minor update it should be 0.6.9.1, otherwise 0.6.10.

Where do you get the idea of a standard for versioning? I could use ISO 8601 of the release date as well for the version string. There is a tradition to use major/minor/micro for Pynac, and I changed micro from 9 to 91 (ninety one) to stay lexicographically consecutive.

The traditional split indicates the use of semantic versioning. In semantic versioning numbers are incremented numerically, not lexicographically. See item (2) in this standard: http://semver.org

comment:10 Changed 3 years ago by rws

Nothing there forbids incrementing by 82.

comment:11 Changed 3 years ago by tscrim

  • Reviewers changed from Paul Masson to Paul Masson, Travis Scrimshaw

Because of this change:

  • src/sage/functions/trig.py

    diff --git a/src/sage/functions/trig.py b/src/sage/functions/trig.py
    index 40c0586..fba1100 100644
    a b class Function_cot(GinacFunction): 
    265265
    266266        TESTS:
    267267
     268            sage: cot(float(0))
     269            Infinity
     270            sage: cot(SR(0))
     271            Infinity
     272            sage: cot(float(0.1))
     273            9.966644423259238
     274            sage: type(_)
     275            <type 'float'>
     276
    268277        Test complex input::
    269278
    270279            sage: cot(complex(1,1))     # rel tol 1e-15

The doc is incorrect. You need to change TESTS: -> TESTS::.

Other than that, I would be willing to set this to a positive review unless Paul has any objections.

comment:12 Changed 3 years ago by git

  • Commit changed from 9634b8a0b3abd225025e4ee6341f58e1114ae531 to 48dac6b87054f77a23936b994c5163f30e381bfa

Branch pushed to git repo; I updated commit sha1. New commits:

48dac6b21623: doc fix

comment:13 Changed 3 years ago by paulmasson

I have a problem with the version number. In semantic versioning, incrementing by 82 implies that many private releases in between versions, which clearly wasn't the case. This version number is inconsistent with the entire versioning history of Pynac.

This version number appears to be an arbitrary choice. As a belatedly recognized direct contributor to this version, I think it is well within my purview to request a modification. As more of the symbolics of Sage is moved into Pynac, I would like to see that project become more of a true collaboration. Changing the version on GitHub is a straightforward process and would give evidence of acceptance of feedback.

I don't think this ticket should be approved with the current version number.

comment:14 Changed 3 years ago by tscrim

I think this is in the realm of bikeshedding.

comment:15 Changed 3 years ago by fbissey

While I disapprove of the numbering, I don't care enough to make it an issue for inclusion/review.

comment:16 Changed 3 years ago by paulmasson

  • Reviewers changed from Paul Masson, Travis Scrimshaw to Travis Scrimshaw

Travis, since there is no review process for Pynac this is the only place to question arbitrary decisions made by one person. If you consider that trivial, so be it.

comment:17 follow-up: Changed 3 years ago by rws

The Pynac review process happens here, nothing new. Maintainer decisions are often seen as arbitrary. I won't devote time to revert a numbering issue. Rather, I started yesterday on fast series expansion using Flint, which will make a new minor number change necessary because of the hard dependency. So if 0.6.91 doesn't get positive review it'll be replaced by 0.7.0 later, anyway. The numbering criticism is noted and will be followed when 0.7.10 is on the plate. Or if.

comment:18 Changed 3 years ago by tscrim

  • Reviewers changed from Travis Scrimshaw to Paul Masson, Travis Scrimshaw
  • Status changed from needs_review to positive_review

IMO, let's try to sneak this into the next Sage release. So I'm setting this to a positive review.

comment:19 in reply to: ↑ 17 Changed 3 years ago by rws

Replying to rws:

...fast series expansion using Flint, which will make a new minor number change necessary because of the hard dependency.

It's done. See #14878.

comment:20 Changed 3 years ago by vbraun

  • Branch changed from u/rws/upgrade_to_pynac_0_6_91 to 48dac6b87054f77a23936b994c5163f30e381bfa
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.