Opened 6 years ago

Closed 6 years ago

Last modified 6 years ago

#19879 closed enhancement (fixed)

Move sage/rings/arith to sage/arith

Reported by: jdemeyer Owned by:
Priority: major Milestone: sage-7.0
Component: basic arithmetic Keywords:
Cc: jpflori, jmantysalo, vbraun Merged in:
Authors: Jeroen Demeyer Reviewers: Jean-Pierre Flori
Report Upstream: N/A Work issues:
Branch: 5e7791c (Commits, GitHub, GitLab) Commit:
Dependencies: #19415 Stopgaps:

Status badges

Description (last modified by jdemeyer)

The file src/sage/rings/arith.py contains a bunch of "arithmetic" functions. I think this does not really belong in rings but in a new top-level directory. Eventually, we should move some more files (integer factorisation, sum of squares, Bernoulli numbers, continued fractions) there but we start with just the file arith.py.

Change History (18)

comment:1 Changed 6 years ago by jdemeyer

  • Summary changed from Move sage/rings/arith to src/arith to Move sage/rings/arith to sage/arith

comment:2 Changed 6 years ago by jdemeyer

  • Description modified (diff)

comment:3 Changed 6 years ago by jdemeyer

  • Authors set to Jeroen Demeyer
  • Dependencies set to #19415

comment:4 Changed 6 years ago by jdemeyer

  • Branch set to u/jdemeyer/move_sage_rings_arith_to_sage_arith

comment:5 Changed 6 years ago by jdemeyer

  • Commit set to 1c3d52d7b8f0341359d1f95e7b35a63e0624424e
  • Status changed from new to needs_review

New commits:

db81fe5Deprecate composite_field()
1c3d52dMove sage.rings.arith to sage.arith

comment:6 Changed 6 years ago by vdelecroix

I think that for the continuation continued_fraction should also move.

Do you mind waiting the next release cycle for starting moving things around?

comment:7 Changed 6 years ago by jdemeyer

  • Description modified (diff)

Do you mind waiting the next release cycle for starting moving things around?

On the contrary, I think that going to a new major release (6.10 -> 7.0) is a very good time to move things around.

But eventually that is the decision of the release manager and it should not affect reviewing this ticket.

comment:8 Changed 6 years ago by jdemeyer

  • Cc jpflori added

comment:9 Changed 6 years ago by git

  • Commit changed from 1c3d52d7b8f0341359d1f95e7b35a63e0624424e to 5e7791c8699fb48811fe4c8a773b33b774a8c443

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:

5e7791cMove sage.rings.arith to sage.arith.misc

comment:10 Changed 6 years ago by jdemeyer

make pteslong passes with this last branch.

comment:11 Changed 6 years ago by jdemeyer

Can somebody please review this? Otherwise this is going to bitrot very quickly. I know it looks like a patch bomb, but all the changes are really trivial.

comment:12 Changed 6 years ago by jmantysalo

  • Cc jmantysalo added

Cc'ing myself because of #19855.

comment:13 Changed 6 years ago by jpflori

  • Reviewers set to Jean-Pierre Flori
  • Status changed from needs_review to positive_review

comment:14 Changed 6 years ago by jdemeyer

Thanks!

comment:15 Changed 6 years ago by jdemeyer

  • Cc vbraun added

Volker, this patch changes a lot of code in Sage. So, in order to avoid endless merge conflicts, would it be possible to either merge this still in 7.0 or very early in 7.1? Thanks.

comment:16 Changed 6 years ago by vbraun

  • Branch changed from u/jdemeyer/move_sage_rings_arith_to_sage_arith to 5e7791c8699fb48811fe4c8a773b33b774a8c443
  • Resolution set to fixed
  • Status changed from positive_review to closed

comment:17 Changed 6 years ago by cremona

  • Commit 5e7791c8699fb48811fe4c8a773b33b774a8c443 deleted

Sorry to comment on a closed ticket, but was anything done to move the documentation for the file arith.py somewhere? I cannot find it. Maybe it was never in the reference manual anyway?

I am interested in this since in #19887 I add a new file file sage/arith which has documentation which I want to be in the ref man.

comment:18 Changed 6 years ago by cremona

OK, I found it in doc/en/reference/rings_standard/index.rst Sorry for the noise.

Note: See TracTickets for help on using tickets.