Opened 9 years ago

Closed 9 years ago

#9901 closed defect (fixed)

update pynac to 0.2.1

Reported by: burcin Owned by: burcin
Priority: major Milestone: sage-4.6
Component: symbolics Keywords: pynac
Cc: mpatel Merged in: sage-4.6.alpha3
Authors: Burcin Erocal Reviewers: Karl-Dieter Crisman
Report Upstream: N/A Work issues:
Branch: Commit:
Dependencies: Stopgaps:

Change History (10)

comment:1 Changed 9 years ago by burcin

  • Description modified (diff)
  • Status changed from new to needs_review

comment:2 Changed 9 years ago by kcrisman

I'm going to try (!) to review all this soon. Just a point: the Pynac website has no mention of this update.

comment:3 Changed 9 years ago by kcrisman

Most of the changesets are pretty obviously related to the tickets in question, and I'll put comments there. Here are a few things which I will put here for archiving which came up in discussion, because a few changesets in Pynac are nonobvious.

> As far as Pynac is concerned:
>
> "Do not generate function.{h,cpp} automatically."  How do I check
> this?  What is the reasoning for this?  How do you test this in the
> Sage library (say, for 13 arguments), as it probably should be?  Why
> do you keep it for (say) 3, but not 4?

You can't check this in the Sage library. The auto generated calls were
something we inherited from GiNaC as a convenience for programming in
C++. Since we use Python, which uses a function call mechanism very
similar to the vector arguments case GiNaC supports, there is no reason
to keep the auto generated functions.

I kept the calls up to 3, since functions implemented in GiNaC use at
least up to 2. I don't know if there is anything which needs 3.

> Are some of these things items you would want to report upstream?    I
> am thinking of the conjugate handling, for instance, or the unsigned
> infinity.  Maybe they come from upstream?

> Changeset 172 - what is that all about?

It just deletes some files from the mercurial repository. They are
generated by autoconf. William just imported everything in the GiNaC
distribution tarball when he started working on pynac. This is just
cleaning up some of that cruft.

Incidentally, we also note that the conjugate handling change came from upstream.

comment:4 Changed 9 years ago by kcrisman

  • Reviewers set to Karl-Dieter Crisman

Update for release manager: positive review for all but #9879 and #9900. #9879 awaits a review of the reviewer patch (just because it's somewhat long), and #9900 will be positive review once the reviewer creates a reviewer patch, hopefully sometime tomorrow.

Also, although the Pynac website doesn't mention these changes yet, the changesets ARE available here, so it is on the web and not just in Sage :)

comment:5 Changed 9 years ago by mpatel

  • Cc mpatel added

comment:6 Changed 9 years ago by jpflori

I only run at the docsets and everything went fine. I'll have a look at #9879 and #9900 later.

comment:7 Changed 9 years ago by kcrisman

  • Status changed from needs_review to positive_review

Based on the now positively reviewed #9879 and #9900, this should have positive review. Hooray!

comment:8 Changed 9 years ago by mpatel

Is there a special order in which I should merge the Pynac tickets?

comment:10 Changed 9 years ago by mpatel

  • Merged in set to sage-4.6.alpha3
  • Resolution set to fixed
  • Status changed from positive_review to closed

Thanks!

Note: See TracTickets for help on using tickets.