Opened 2 years ago
Last modified 18 months ago
#24457 needs_work task
modernize sage.rings.real_mpfr
Reported by:  vdelecroix  Owned by:  

Priority:  major  Milestone:  sage8.4 
Component:  basic arithmetic  Keywords:  
Cc:  Merged in:  
Authors:  Reviewers:  
Report Upstream:  N/A  Work issues:  
Branch:  Commit:  
Dependencies:  Stopgaps: 
Description (last modified by )
The current sage.rings.real_mpfr
deals with arbitrary precision floatingpoint numbers (using mpfr). This is the default "real field" in many places in Sage even though it is often wiser to use intervals or balls. We perform some cosmetic changes to sage.rings.real_mpfr
to emphasize the fact that the module deals with floatingpoint approximations of real numbers and not genuine reals. These modifications touch a lot of files and involve deprecation of widely used names.
See also the introduction of a genuine real field class in #24456 for another motivation.
step 1
 #24511: move the class factory
create_RealField
inreal_field.py
 #24523: change string representation
 #24524:
RealField
>RealFloatingPointField_factory
RealField_class
>RealFloatingPointField
RealNumber
>RealFloatingPoint
 #24525: deprecate
is_RealNumber(x)
andis_RealField(x)
in favour ofisinstance(x, RealFloatingPoint)
andisinstance(x, RealFloatingPointField)
step 2
 Deprecate
RR
andReals
and useRFF
(orRFPF
?) as the standard name forRealFloatingPointField(53)
?  more documentation.
for complex numbers, see #24489.
Change History (51)
comment:1 Changed 2 years ago by
 Branch #24456 deleted
 Dependencies set to #24456
comment:2 Changed 2 years ago by
comment:3 Changed 2 years ago by
 Description modified (diff)
comment:4 Changed 2 years ago by
 Description modified (diff)
comment:5 Changed 2 years ago by
 Branch set to u/rws/rename_sage_rings_real_mpfr_realfield
comment:6 Changed 2 years ago by
 Commit set to 71f9fd0b38f89ca57fa70f88bc80fa62d124322c
 Status changed from new to needs_review
comment:7 Changed 2 years ago by
Ah I forgot the string representation part.
comment:8 Changed 2 years ago by
 Branch u/rws/rename_sage_rings_real_mpfr_realfield deleted
 Commit 71f9fd0b38f89ca57fa70f88bc80fa62d124322c deleted
 Status changed from needs_review to needs_work
Sorry, I was pretty sure I excluded real_field.py
.
comment:9 Changed 2 years ago by
 Branch set to u/rws/244571
comment:10 Changed 2 years ago by
 Commit set to 8bdf66b2d1e120977cb3b0c5a04bdbfcf988a465
 Status changed from needs_work to needs_review
comment:11 in reply to: ↑ description ; followup: ↓ 15 Changed 2 years ago by
Replying to vdelecroix:
 change the name of
RealField
/RealField_class
toRealFloatingPointField
/RealFloatingPointField_class
Do we still need the old RealField
given that it's no longer used after #24456?
comment:12 Changed 2 years ago by
 Status changed from needs_review to needs_work
comment:13 Changed 2 years ago by
 Branch changed from u/rws/244571 to u/rws/244572
comment:14 Changed 2 years ago by
 Commit changed from 8bdf66b2d1e120977cb3b0c5a04bdbfcf988a465 to 3c7f16e17aa989aa2d0eeef196391a04bf1b0192
 Status changed from needs_work to needs_review
New commits:
3c7f16e  24457: rename sage.rings.real_mpfr.RealField

comment:15 in reply to: ↑ 11 ; followup: ↓ 16 Changed 2 years ago by
Replying to jdemeyer:
Replying to vdelecroix:
 change the name of
RealField
/RealField_class
toRealFloatingPointField
/RealFloatingPointField_class
Do we still need the old
RealField
given that it's no longer used after #24456?
It is. The old real field is the field of floating point numbers that is intensively used (and renamed in this ticket). The new real field is the field of real numbers (the real one).
comment:16 in reply to: ↑ 15 ; followups: ↓ 17 ↓ 19 Changed 2 years ago by
Replying to vdelecroix:
The old real field is the field of floating point numbers that is intensively used (and renamed in this ticket).
I was talking about the RealField()
function. Is that still used?
If so, I would rather rename it to create_RealFloatingPointField
.
comment:17 in reply to: ↑ 16 ; followup: ↓ 18 Changed 2 years ago by
Replying to jdemeyer:
Replying to vdelecroix:
The old real field is the field of floating point numbers that is intensively used (and renamed in this ticket).
I was talking about the
RealField()
function. Is that still used?If so, I would rather rename it to
create_RealFloatingPointField
.
I think that we should get rid of this function and the hand made cache. The caching would better be handled by UniqueRepresentation
by appropriately writing __classcall__
/__init__
. This class is very old.
comment:18 in reply to: ↑ 17 Changed 2 years ago by
Replying to vdelecroix:
Replying to jdemeyer:
Replying to vdelecroix:
The old real field is the field of floating point numbers that is intensively used (and renamed in this ticket).
I was talking about the
RealField()
function. Is that still used?If so, I would rather rename it to
create_RealFloatingPointField
.I think that we should get rid of this function and the hand made cache. The caching would better be handled by
UniqueRepresentation
by appropriately writing__classcall__
/__init__
. This class is very old.
But this is rather disjoint from this ticket.
comment:19 in reply to: ↑ 16 ; followup: ↓ 20 Changed 2 years ago by
Replying to jdemeyer:
Replying to vdelecroix:
The old real field is the field of floating point numbers that is intensively used (and renamed in this ticket).
I was talking about the
RealField()
function. Is that still used?If so, I would rather rename it to
create_RealFloatingPointField
.
For the sake of this ticket +1.
comment:20 in reply to: ↑ 19 ; followup: ↓ 21 Changed 2 years ago by
Replying to vdelecroix:
Replying to jdemeyer:
If so, I would rather rename it to
create_RealFloatingPointField
.For the sake of this ticket +1.
You mean "in this ticket"?
comment:21 in reply to: ↑ 20 ; followup: ↓ 22 Changed 2 years ago by
Replying to rws:
Replying to vdelecroix:
Replying to jdemeyer:
If so, I would rather rename it to
create_RealFloatingPointField
.For the sake of this ticket +1.
You mean "in this ticket"?
Sorry for my english. Yes, this is what I meant.
comment:22 in reply to: ↑ 21 Changed 2 years ago by
Replying to jdemeyer:
If so, I would rather rename it to
create_RealFloatingPointField
.
Ah I see, in rings/all.py
: import create_RealFloatingPointField as RealFloatingPointField
.
comment:23 Changed 2 years ago by
 Branch changed from u/rws/244572 to u/rws/244573
comment:24 Changed 2 years ago by
 Branch changed from u/rws/244573 to u/rws/244574
comment:25 Changed 2 years ago by
 Commit changed from 3c7f16e17aa989aa2d0eeef196391a04bf1b0192 to 9b2a5c2a38cf20b52dab663cecffc1009eb6ecbd
Branch pushed to git repo; I updated commit sha1. New commits:
9b2a5c2  24457: missing changes in interpreters

comment:26 followup: ↓ 28 Changed 2 years ago by
Some quick comments:
 why renaming
RealField
and notRR
at the same time ?  i am not sure that completely changing the behaviour of
RealField
without any kind of deprecation period is wise, how will the user's code survive to this change ?  do you really plan to change a behaviour that implies modifying 164 files without even an email to sagedevel ?
 Regarding the naming, i would personally prefer
Floatingpoint
overFloating Point
(as it is usually), moreover, it has the advantage that the shortcut will beRFF
(notRFPF
), consistently with other representations of real numbers.  where is the doc ?
comment:27 Changed 2 years ago by
 Status changed from needs_review to needs_work
It's in the experimental phase. I will need an IDE with refactoring capabilities to do this reliably and quickly, or develop scripts. Also, I think it will need its own beta.
comment:28 in reply to: ↑ 26 Changed 2 years ago by
Replying to tmonteil:
Some quick comments:
 why renaming
RealField
and notRR
at the same time ?
why not. But it will
 be a much more delicate backward incompatibility.
 make the ticket even bigger
The 2 steps transition seems simpler to me.
 i am not sure that completely changing the behaviour of
RealField
without any kind of deprecation period is wise, how will the user's code survive to this change ?
There must be deprecation about the name changes (which is not the case in the current branch). RealField
shoud still point to the FloatingPointField
(with deprecation) and RR
should still point to RFF
(with deprecation).
I don't think there is a need for deprecation about the change of printing.
 do you really plan to change a behaviour that implies modifying 164 files without even an email to sagedevel ?
 Regarding the naming, i would personally prefer
Floatingpoint
overFloating Point
(as it is usually), moreover, it has the advantage that the shortcut will beRFF
(notRFPF
), consistently with other representations of real numbers.
+1 for Floatingpoint.
 where is the doc ?
comment:29 Changed 2 years ago by
 Dependencies #24456 deleted
 Description modified (diff)
comment:30 Changed 2 years ago by
 Description modified (diff)
comment:31 Changed 2 years ago by
 Branch u/rws/244574 deleted
 Commit 9b2a5c2a38cf20b52dab663cecffc1009eb6ecbd deleted
comment:32 Changed 2 years ago by
 Description modified (diff)
 Summary changed from rename sage.rings.real_mpfr.RealField to modernize sage.rings.real_mpfr
comment:33 Changed 2 years ago by
I'm confused. You deleted the description item that renames RealField/RealField_class
in comment:29. Does that mean those refactorings are not planned in this ticket? On the other hand you write isinstance(x, RealFloatingPointField)
so the new name is used?
As to automating this and other big refactorings in Python Sage I just learned that 1. Pycharm refactoring apparently cannot be scripted (not to be expected anyway in an IDE), and 2. the rope tool is not up to the task.
comment:34 Changed 2 years ago by
 Description modified (diff)
comment:35 Changed 2 years ago by
 Dependencies set to #24511
 Description modified (diff)
comment:36 followup: ↓ 37 Changed 2 years ago by
 Description modified (diff)
The deprecation of is_RealNumber(x)
/is_RealField(x)
in favor of isinstance
can be done independently of the renaming. Separate ticket to come.
comment:37 in reply to: ↑ 36 ; followup: ↓ 42 Changed 2 years ago by
Replying to rws:
The deprecation of
is_RealNumber(x)
/is_RealField(x)
in favor ofisinstance
can be done independently of the renaming. Separate ticket to come.
Good idea!
comment:38 Changed 2 years ago by
 Description modified (diff)
Note that is_RealField(X)
then becomes isinstance(X, RealField_class)
.
comment:39 Changed 2 years ago by
 Description modified (diff)
comment:40 Changed 2 years ago by
 Description modified (diff)
 Type changed from enhancement to task
comment:41 Changed 2 years ago by
Let us do that incrementally (ie each subtask in ticket). It will simplify review and merging. Though I would feel much more confortable if the whole task would be actually closed within the next stable release.
comment:42 in reply to: ↑ 37 Changed 2 years ago by
Replying to vdelecroix:
Replying to rws:
The deprecation of
is_RealNumber(x)
/is_RealField(x)
in favor ofisinstance
can be done independently of the renaming. Separate ticket to come.Good idea!
Though it would be better to first change the names:
RealField
>RealFloatingPointField_factory
RealField_class
>RealFloatingPointField
RealNumber
>RealFloatingPoint
to avoid passing twice on the same lines.
comment:43 Changed 2 years ago by
 Description modified (diff)
comment:44 Changed 2 years ago by
 Description modified (diff)
comment:45 Changed 2 years ago by
 Description modified (diff)
comment:46 Changed 2 years ago by
 Description modified (diff)
comment:47 Changed 2 years ago by
 Description modified (diff)
comment:48 Changed 2 years ago by
 Status changed from needs_work to needs_review
Rebased on the remaining dependency.
comment:50 Changed 20 months ago by
 Dependencies #24511 deleted
 Milestone changed from sage8.2 to sage8.3
comment:51 Changed 18 months ago by
 Milestone changed from sage8.3 to sage8.4
update milestone 8.3 > 8.4
In
real_mpfr.pyx
do you mean to renamecpdef RealField(int prec=53,...)
orcdef class RealField_class
?