Opened 3 years ago

Closed 20 months ago

#20310 closed enhancement (fixed)

change for p-adic rings

Reported by: caruso Owned by:
Priority: major Milestone: sage-7.2
Component: padics Keywords: precision
Cc: saraedum, roed Merged in:
Authors: David Roe Reviewers: Julian Rüth, Xavier Caruso
Report Upstream: N/A Work issues:
Branch: 86b58de (Commits) Commit: 86b58de413e57a646288c2d0ac38db75e6d04430
Dependencies: #23331 Stopgaps:

Description (last modified by saraedum)

Started as a proposal to change precision of rings, this is now a more general change method.

Attachments (1)

20310_over_23331.diff (64.1 KB) - added by roed 20 months ago.
Diff against 23331

Download all attachments as: .zip

Change History (63)

comment:1 Changed 3 years ago by caruso

As discussed in Sage Days 71, I tried to implement this method using the construction functor but it seems that the latter is only defined for Zp or Qp but not for extensions. So, what should I do? Implement the methods construction as well? Something else?

comment:2 Changed 3 years ago by caruso

  • Branch set to u/caruso/change_precision

comment:3 Changed 3 years ago by roed

  • Commit set to 9ca8df8885d967a6b36bb6cb89fb79484cc25563

Implementing construction is a good idea anyway, since they're used in coercion. And until we change extensions to be defined by exact polynomials, you'll need to raise the precision of the base ring as well, so it's probably necessary.


New commits:

9ca8df8Generic function change_precision

comment:4 Changed 22 months ago by roed

  • Branch changed from u/caruso/change_precision to u/roed/change_precision

comment:5 Changed 22 months ago by git

  • Commit changed from 9ca8df8885d967a6b36bb6cb89fb79484cc25563 to 53d24469801c5ccea6b13b59a30ecd11eafa4d29

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

e16f79fAdd construction method for extensions of p-adic rings and fields, switch implementation of fraction_field and integer_ring
a88b630Fixing problems revealed by tests
fdd03cbMerge branch 'u/roed/padic-floats' of git://trac.sagemath.org/sage into t/20310/change_precision
53d2446Added ability to edit show_prec to p-adic factory

comment:6 Changed 22 months ago by git

  • Commit changed from 53d24469801c5ccea6b13b59a30ecd11eafa4d29 to 55c335efa39a5f9a5b9808a5f86a4017702de522

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

55c335eFixing doctest problems in the new change method for p-adic rings/fields

comment:7 Changed 21 months ago by git

  • Commit changed from 55c335efa39a5f9a5b9808a5f86a4017702de522 to fcc8aa5279999dcadf48650897a6b19547952b19

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

fcc8aa5Fixing various problems with the new change method

comment:8 Changed 21 months ago by git

  • Commit changed from fcc8aa5279999dcadf48650897a6b19547952b19 to 3b2d94a15e0de51fa5ba2419fa53cbfe5b4f810d

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

3b2d94aRemove old code for fraction_field and integer_ring from padic_extension_generic

comment:9 Changed 21 months ago by git

  • Commit changed from 3b2d94a15e0de51fa5ba2419fa53cbfe5b4f810d to f386dc614c3369f41cdde8b8057a10025464998e

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

f386dc6Fix bug in ring_of_integers

comment:10 Changed 21 months ago by roed

  • Status changed from new to needs_review

comment:11 Changed 21 months ago by saraedum

  • Description modified (diff)
  • Summary changed from change_precision for p-adic rings to change for p-adic rings

comment:12 Changed 21 months ago by caruso

I'm a bit sceptical about the possibility of changing p (and even q). Can you show me a situation where this can be needed?

Adding show_prec is nice but is it really the subject of this ticket?

Doctests implying the keyword base are missing. The same is true for show_prec. More generally, I suggest to add many more doctests ensuring that your code works in a wide range of situations (for ramified, unramified extensions, for extensions constructing via CompletionFunction and AlgebraicExtensionFunctor, etc.)

comment:13 Changed 21 months ago by saraedum

Xavier: Thanks for having a look at this as well. We discussed the changing of p and q briefly and actually I find it very useful. Often I am only really interested in the ramified part of a tower of extensions (on my fork that already does more general towers.) But for some computation to work out, I need a certain unramified element at the bottom, so I don't want to manually reconstruct the whole tower, so raising and lowering q is quite useful. Changing p is mostly interesting in general extensions where you want to see how a certain polynomial behaves wrt to different primes. For the Eisenstein extensions that we support atm it is not too useful.

I think it's Ok to put the show_prec in here, I'm fine with reviewing it at least :)

I agree that doctests for base and show_prec should be added. However, I think that in general not all possible cases should be covered by explicit doctests. It usually makes the documentation overwhelming. If we want to have coverage, we should write unit tests for that instead. That's btw one of the topics that we want to push in Burlington! Imho, the testing for change is Ok except for the untested keywords.

comment:14 Changed 21 months ago by saraedum

David: should get_key_base set show_prec? Otherwise, show_prec=None and show_prec=True/False produces two non-identical parents for the same ring I think.

comment:15 Changed 21 months ago by saraedum

  • Authors set to David Roe
  • Reviewers set to Julian Rüth
  • Status changed from needs_review to needs_work
  • Work issues set to show_prec in factory keys

comment:16 Changed 21 months ago by git

  • Commit changed from f386dc614c3369f41cdde8b8057a10025464998e to f4a534528ce461178ed32e60266beb0f28db00dc

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

f4a5345Move show prec logic into get_key

comment:17 Changed 21 months ago by roed

  • Status changed from needs_work to needs_review
  • Work issues show_prec in factory keys deleted

I fixed the show_prec issue.

comment:18 Changed 21 months ago by saraedum

Looks good to me. Feel free to set this to positive review if the tests pass.

comment:19 Changed 21 months ago by caruso

  • Status changed from needs_review to needs_work
sage: R = Zp(5)
---------------------------------------------------------------------------
TypeError                                 Traceback (most recent call last)
<ipython-input-1-1247c7c70bf9> in <module>()
----> 1 R = Zp(Integer(5))

/home/xavier/sage/src/sage/structure/factory.pyx in sage.structure.factory.UniqueFactory.__call__ (/home/xavier/sage/src/build/cythonized/sage/structure/factory.c:1728)()
    360             False
    361         """
--> 362         key, kwds = self.create_key_and_extra_args(*args, **kwds)
    363         version = self.get_version(sage_version)
    364         return self.get_object(version, key, kwds)

/home/xavier/sage/src/sage/structure/factory.pyx in sage.structure.factory.UniqueFactory.create_key_and_extra_args (/home/xavier/sage/src/build/cythonized/sage/structure/factory.c:2921)()
    458             ((3, ('x',), None, 'modn', "{'foo': 'value'}", 3, 1, True), {'foo': 'value'})
    459         """
--> 460         return self.create_key(*args, **kwds), {}
    461 
    462     def create_key(self, *args, **kwds):

/home/xavier/sage/local/lib/python2.7/site-packages/sage/rings/padics/factory.pyc in create_key(self, p, prec, type, print_mode, halt, names, ram_name, print_pos, print_sep, print_alphabet, print_max_terms, show_prec, check)
   1608         """
   1609         return get_key_base(p, prec, type, print_mode, halt, names, ram_name, print_pos, print_sep, print_alphabet,
-> 1610                             print_max_terms, show_prec, check, ['capped-rel', 'fixed-mod', 'capped-abs', 'floating-point'])
   1611 
   1612     def create_object(self, version, key):

/home/xavier/sage/local/lib/python2.7/site-packages/sage/rings/padics/factory.pyc in get_key_base(p, prec, type, print_mode, halt, names, ram_name, print_pos, print_sep, print_alphabet, print_max_terms, show_prec, check, valid_non_lazy_types)
    161         if type == 'floating-point':
    162             show_prec = False
--> 163         elif print_mode ('series', 'terse', 'val-unit'):
    164             show_prec = True
    165         else:

TypeError: 'str' object is not callable

comment:20 Changed 21 months ago by caruso

By the way, I guess that your changes is not compatible with ticket #22103.

It's of course not that annoying (and, as I've already said, I'm fine with adding the keyword show_prec) but maybe we could have a discussion about the "right way" to print p-adic numbers.

comment:21 Changed 21 months ago by git

  • Commit changed from f4a534528ce461178ed32e60266beb0f28db00dc to e376b9027401c2e2b83f659f9958aac374d747ea

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

e376b90Fix doctest errors and a bug in change related to show_prec

comment:22 Changed 21 months ago by roed

  • Status changed from needs_work to needs_review

I just fixed various doctest issues and added some code for the case of changing print_mode (show_prec was not changed). I ran all tests on the result, and they all pass, but I think I need someone to take a look rather than just setting it to positive review.

comment:23 Changed 21 months ago by caruso

  • Status changed from needs_review to needs_work

Well, I'm still not that convinced by changing p. You say that "it is mostly interesting in general extensions where you want to see how a certain polynomial behaves wrt to different primes" but I guess that the modulus is just stored modulo p^prec, so converting it in others Z_p's may lead to very unexpected behaviours, no? I nevertheless agree that changing q (keeping the same p) makes more sense.

Anyway, changing p actually does not work in many cases:

sage: R.<a> = Zq(5^3)
sage: R.change(p=7)
...
NotImplementedError: conversion between padic extensions not implemented

For ramified extensions, something very stange happens:

sage: R.<a> = Zp(5).extension(x^3 + 5)
sage: R
Eisenstein Extension of 5-adic Ring with capped relative precision 20 in a defined by  (1 + O(5^20))*x^3 + (O(5^21))*x^2 + (O(5^21))*x + (5 + O(5^21))

sage: S = R.change(p=7)
sage: S
Unramified Extension of 7-adic Ring with capped relative precision 20 in a defined by (1 + O(5^20))*x^3 + (O(5^20))*x^2 + (O(5^20))*x + (5 + O(5^20))

Look at the modulus, it is still written with some O(5^20)'s (whereas we are supposed to be on a 7-adic ring). Even more stranger:

sage: c = S.modulus()[0]; c
5 + O(5^20)
sage: c.parent()
7-adic Ring with capped relative precision 20

sage: c.parent() is Zp(7)
False

As a conclusion, I would be in favour to remove this feature.

comment:24 Changed 21 months ago by caruso

Other issues with the change of precision:

sage: R.<a> = Zq(5^3)
sage: S = R.change(prec=50)
sage: S
Unramified Extension of 5-adic Ring with capped relative precision 50 in a defined by (1 + O(5^20))*x^3 + (O(5^20))*x^2 + (3 + O(5^20))*x + (3 + O(5^20))

The defining polynomial has not been lifted to precision 50.

In fact, it seems that we cannot compute at all at precision 50 is S:

sage: S.gen()
a + O(5^20)
sage: S.precision_cap()
20

For ramified extensions, the precision on the modulus decreases:

sage: R.<a> = Zp(5).extension(x^3 + 5)
sage: R
Eisenstein Extension of 5-adic Ring with capped relative precision 20 in a defined by (1 + O(5^20))*x^3 + (O(5^21))*x^2 + (O(5^21))*x + (5 + O(5^21))
sage: S = R.change(prec=50)
sage: S
Eisenstein Extension of 5-adic Ring with capped relative precision 20 in a defined by (1 + O(5^18))*x^3 + (O(5^18))*x^2 + (O(5^18))*x + (5 + O(5^18))

but (unexpectedly) computations in S seem to be fine.

comment:25 Changed 21 months ago by roed

Good points. We've also been thinking about switching defining polynomials to being exact, in which case some of the precision issues go away.

I'll take a look at this when I'm back, in a week and a half or so.

comment:26 Changed 21 months ago by git

  • Commit changed from e376b9027401c2e2b83f659f9958aac374d747ea to 543088a49d66f2927741c086f3271f30c4ff86a8

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

543088aFix broken printing when changing p in a p-adic extension, raise error when creating an extension with precision larger than the precision of the defining polynomial

comment:27 Changed 21 months ago by git

  • Commit changed from 543088a49d66f2927741c086f3271f30c4ff86a8 to b643d244032f32ef68692aa43e805ca9bc0eb206

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

b643d24Delay substitution of DEFAULT_PREC in padics factory

comment:28 Changed 21 months ago by git

  • Commit changed from b643d244032f32ef68692aa43e805ca9bc0eb206 to e935ca9f842d729f349b34f058e647a55068d0f1

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

e935ca9Change Zq and Qq to be defined by an exact polynomial

comment:29 Changed 21 months ago by roed

  • Status changed from needs_work to needs_review

I fixed the printing problem that was causing the weird behavior you observed when changing p. If the defining polynomial is defined over the integers, then changing p works; if not it doesn't.

I also added an error if you try to increase the precision above the limit imposed by the defining polynomial.

comment:30 Changed 21 months ago by git

  • Commit changed from e935ca9f842d729f349b34f058e647a55068d0f1 to 547b66d756630f5e68b01b3f31b460ea4b3d9f13

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

547b66dFix error in check=False constructor for Qq

comment:31 Changed 21 months ago by roed

All tests pass for me.

comment:32 follow-up: Changed 21 months ago by caruso

  • Status changed from needs_review to needs_work

Not a complete review, but I noticed the following:

In the doctest of the method change, some items are introduced by -- instead of -.

When creating an unramified extension, we cannot specify the variable name using the keyword unram_name:

sage: R = Zp(5)
sage: R.change(q=25, unram_name='a')
...
TypeError: You must specify the name of the generator

sage: R2.<a> = Zq(5^2)
sage: R2.change(q=125,unram_name='b')
Unramified Extension of 5-adic Ring with capped relative precision 20 in a defined by (1 + O(5^20))*x^3 + (O(5^20))*x^2 + (3 + O(5^20))*x + (3 + O(5^20))

(Note nevertheless than using names instead of unram_name works fine.)

I am a bit lost with exact VS inexact defining polynomials. For instance:

sage: var('x')
x
sage: R.<a> = Zq(5^2, modulus=x^2+2*x+4)
sage: R.change(p=11)
Unramified Extension of 11-adic Ring with capped relative precision 20 in a defined by (1 + O(11^20))*x^2 + (2 + O(11^20))*x + (4 + O(11^20))

sage: ZX.<x> = ZZ[]
sage: R.<a> = Zq(5^2, modulus=x^2+2*x+4)
sage: R.change(p=11)
...
NotImplementedError: conversion between padic extensions not implemented

By the way, even when the defining polynomial is exact, the methods modulus and defining_polynomial return an inexact polynomial. Is there a method that returns the exact defining polynomial (i.e. the content of the attribute _pre_poly as far as I understand)?

comment:33 Changed 21 months ago by git

  • Commit changed from 547b66d756630f5e68b01b3f31b460ea4b3d9f13 to bf8d7606b21ecbbcdebaf9a96e3f5f2befe1756c

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

bf8d760Fix formatting errors in change docstring, allow specifying unram_name rather than names in changing q on Zp or Qp

comment:34 in reply to: ↑ 32 Changed 21 months ago by roed

Replying to caruso:

Not a complete review, but I noticed the following:

In the doctest of the method change, some items are introduced by -- instead of -.

Fixed, thanks.

When creating an unramified extension, we cannot specify the variable name using the keyword unram_name:

sage: R = Zp(5)
sage: R.change(q=25, unram_name='a')
...
TypeError: You must specify the name of the generator

sage: R2.<a> = Zq(5^2)
sage: R2.change(q=125,unram_name='b')
Unramified Extension of 5-adic Ring with capped relative precision 20 in a defined by (1 + O(5^20))*x^3 + (O(5^20))*x^2 + (3 + O(5^20))*x + (3 + O(5^20))

(Note nevertheless than using names instead of unram_name works fine.)

Fixed.

I am a bit lost with exact VS inexact defining polynomials. For instance:

sage: var('x')
x
sage: R.<a> = Zq(5^2, modulus=x^2+2*x+4)
sage: R.change(p=11)
Unramified Extension of 11-adic Ring with capped relative precision 20 in a defined by (1 + O(11^20))*x^2 + (2 + O(11^20))*x + (4 + O(11^20))

sage: ZX.<x> = ZZ[]
sage: R.<a> = Zq(5^2, modulus=x^2+2*x+4)
sage: R.change(p=11)
...
NotImplementedError: conversion between padic extensions not implemented

I get that both of these work. It's possible that your cache has been poisoned by trying to first create R using a p-adic modulus, and then later with a modulus over Z? This is the kind of thing that might be alleviated by #23331.

By the way, even when the defining polynomial is exact, the methods modulus and defining_polynomial return an inexact polynomial. Is there a method that returns the exact defining polynomial (i.e. the content of the attribute _pre_poly as far as I understand)?

I don't think there's currently a method to get _pre_poly. I would suggest holding off on that change until #23331.

comment:35 Changed 21 months ago by caruso

I get that both of these work. It's possible that your cache has been poisoned by trying to first create R using a p-adic modulus, and then later with a modulus over Z? This is the kind of thing that might be alleviated by #23331.

Good point. With a fresh session of sage, I have:

sage: R.<a> = Zq(5^2, modulus=x^2+2*x+4)
sage: R.change(p=11)
Unramified Extension of 11-adic Ring with capped relative precision 20 in a defined by (1 + O(11^20))*x^2 + (2 + O(11^20))*x + (4 + O(11^20))
sage: 
sage: ZX.<x> = ZZ[]
sage: R.<a> = Zq(5^2, modulus=x^2+2*x+4)
sage: R.change(p=11)
Unramified Extension of 11-adic Ring with capped relative precision 20 in a defined by (1 + O(11^20))*x^2 + (2 + O(11^20))*x + (4 + O(11^20))

but:

sage: ZpX.<x> = Zp(5)[]
sage: R1.<a> = Zq(5^2, modulus=x^2+2*x+4)
sage: R1.change(p=11)
...
NotImplementedError: conversion between padic extensions not implemented

sage: ZX.<x> = ZZ[]
sage: R2.<a> = Zq(5^2, modulus=x^2+2*x+4)
sage: R2.change(p=11)
...
NotImplementedError: conversion between padic extensions not implemented

and conversely:

sage: ZX.<x> = ZZ[]
sage: R2.<a> = Zq(5^2, modulus=x^2+2*x+4)
sage: R2.change(p=11)
Unramified Extension of 11-adic Ring with capped relative precision 20 in a defined by (1 + O(11^20))*x^2 + (2 + O(11^20))*x + (4 + O(11^20))

sage: ZpX.<x> = Zp(5)[]
sage: R1.<a> = Zq(5^2, modulus=x^2+2*x+4)
sage: R1.change(p=11)
Unramified Extension of 11-adic Ring with capped relative precision 20 in a defined by (1 + O(11^20))*x^2 + (2 + O(11^20))*x + (4 + O(11^20))

I guess that the problem comes from the factory because, in the above examples, R1 is the same as R2 (i.e. R1 is R2 answers True).

comment:36 Changed 21 months ago by roed

  • Reviewers changed from Julian Rüth to Julian Rüth, Xavier Caruso
  • Status changed from needs_work to needs_review

comment:37 Changed 21 months ago by saraedum

  • Status changed from needs_review to positive_review

Unless Xavier has any objections of course.

comment:38 Changed 21 months ago by caruso

  • Status changed from positive_review to needs_work

Well, I am a bit worried to give a positive review after comment 35 (which is clearly a bug for me).

I am fine with fixing this in ticket #23331 but, in that case, I would wait for #23331 to be written and positively reviewed before giving a positive review to the current ticket (#20310)

comment:39 Changed 21 months ago by saraedum

I don't see how this is related to this ticket really. This is just a bug in the factory that has always been there. The same problem at least happens on a stable sage

sage: ZpX.<x> = Zp(5)[]
sage: R1.<a> = Zq(5^2, modulus=x^2+2*x+4)
sage: ZX.<x> = ZZ[]
sage: R2.<a> = Zq(5^2, modulus=x^2+2*x+4)
sage: R1 is R2
True

I guess my point is that you could use this to construct problems with many methods in Sage. I do not see why we should have change() be blocked by this now. (We should of course fix this, nevertheless…)

comment:40 Changed 21 months ago by saraedum

  • Status changed from needs_work to needs_review

comment:41 Changed 21 months ago by caruso

I do not agree. This was not a bug until this ticket implemented a method, namely change, that decides to treat differently extensions defined by exact and inexact polynomials.

comment:42 Changed 21 months ago by saraedum

  • Dependencies set to #23331
  • Status changed from needs_review to positive_review

Ok. I guess you're right that this was much more unlikely to cause trouble before.

I still don't think that this should block this ticket, but I guess we have to wait for #23331 then.

comment:43 Changed 20 months ago by git

  • Commit changed from bf8d7606b21ecbbcdebaf9a96e3f5f2befe1756c to 8351aa992136b9228983755cdb1cda0e7dbd315f
  • Status changed from positive_review to needs_review

Branch pushed to git repo; I updated commit sha1 and set ticket back to needs_review. New commits:

7e21e77Adding an exact modulus to p-adic rings and fields, changing printing for p-adic extensions, removing some vestigal lazy p-adic code
dbbc631Fix pickles
6cb9d22Remove extraneous newlines in padic_ZZ_pX_FM_element.pyx
daf51faMerge branch 'u/roed/change_precision' of git://trac.sagemath.org/sage into t/20310/change_precision
8351aa9Fixing doctest errors from merge

comment:44 Changed 20 months ago by roed

I merged in the dependency #23331; all tests pass.

comment:45 Changed 20 months ago by git

  • Commit changed from 8351aa992136b9228983755cdb1cda0e7dbd315f to 85019cc687930cd9be60d01e45eb08dd9ea91910

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

4ac09acFix changing print mode to digits for eisenstein extensions
85019ccFix errors due to moving digits code earlier in the change function

comment:46 Changed 20 months ago by git

  • Commit changed from 85019cc687930cd9be60d01e45eb08dd9ea91910 to 39043f13cc8d6d6eeda3a639b7a3566c27b5b834

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

7f96697Fixed typo with variable assignment
bbb7268added doctests
ee461b6merge with develop
8081eabadded doctests after fixing conflicts
063b58cMerge branch 'develop' into t/20073/ticket/20073
48be601Added documentation to verify that the extension has the right defining polynomial
921af5eChanging modulus and defining_polynomial to use an exact keyword
39043f1Merge branch 't/23331/allow_exact_defining_polynomials_for_p_adic_extensions' into t/20310/change_precision

comment:47 Changed 20 months ago by saraedum

  • Branch changed from u/roed/change_precision to u/saraedum/change_precision

comment:48 Changed 20 months ago by saraedum

  • Commit changed from 39043f13cc8d6d6eeda3a639b7a3566c27b5b834 to 7428353cf0520558fb24b292e48be1358fa0d305

positive review once the patchbot is happy.


New commits:

77779eaminor docstring changes
6e2495fMerge branch 't/23331/allow_exact_defining_polynomials_for_p_adic_extensions' into t/20310/change_precision
7428353minor docstring fixes

comment:49 Changed 20 months ago by roed

  • Branch changed from u/saraedum/change_precision to u/roed/change_precision

comment:50 Changed 20 months ago by git

  • Commit changed from 7428353cf0520558fb24b292e48be1358fa0d305 to 1754b445f154e165e2545af99a5882da8cdcb01e

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

1754b44Fix exact=True errors in the right branch

comment:51 Changed 20 months ago by roed

  • Status changed from needs_review to positive_review

comment:52 Changed 20 months ago by git

  • Commit changed from 1754b445f154e165e2545af99a5882da8cdcb01e to 3142701a2bd3b2e98601d521df2b1111cec1537d
  • Status changed from positive_review to needs_review

Branch pushed to git repo; I updated commit sha1 and set ticket back to needs_review. New commits:

6ba62ddFix SEEALSO again
e9c4c39Merge branch 'u/roed/allow_exact_defining_polynomials_for_p_adic_extensions' of git://trac.sagemath.org/sage into t/23331/allow_exact_defining_polynomials_for_p_adic_extensions
561f5acFix doctest errors
3142701Merge branch 'u/roed/change_precision' of git://trac.sagemath.org/sage into t/20310/change_precision

comment:53 Changed 20 months ago by roed

  • Status changed from needs_review to positive_review

I just merged in the updated #23331.

comment:54 Changed 20 months ago by git

  • Commit changed from 3142701a2bd3b2e98601d521df2b1111cec1537d to 138d939114acf0943561e05b194d677b63879bb2
  • Status changed from positive_review to needs_review

Branch pushed to git repo; I updated commit sha1 and set ticket back to needs_review. New commits:

138d939Fix string representation doctest from #22103

comment:55 Changed 20 months ago by roed

  • Status changed from needs_review to positive_review

Changed 20 months ago by roed

Diff against 23331

comment:56 Changed 20 months ago by roed

For ease of reviewing, I've attached a diff against #23331.

Last edited 20 months ago by roed (previous) (diff)

comment:57 Changed 20 months ago by vbraun

  • Status changed from positive_review to needs_work

PDF docs don't build

comment:58 Changed 20 months ago by git

  • Commit changed from 138d939114acf0943561e05b194d677b63879bb2 to b18929a1eab29a29b5c943a1d8b1b7d649f2dfb2

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

b18929aMerge branch 'develop' into t/20310/change_precision

comment:59 Changed 20 months ago by git

  • Commit changed from b18929a1eab29a29b5c943a1d8b1b7d649f2dfb2 to 86b58de413e57a646288c2d0ac38db75e6d04430

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

86b58deFix latex error

comment:60 Changed 20 months ago by roed

  • Status changed from needs_work to needs_review

Alright, make succeeds at building the docs now.

comment:61 Changed 20 months ago by saraedum

  • Status changed from needs_review to positive_review

daucher still reports a failing docbuild. I guess that's because I told it not to clean the docs when rebuilding.

comment:62 Changed 20 months ago by vbraun

  • Branch changed from u/roed/change_precision to 86b58de413e57a646288c2d0ac38db75e6d04430
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.