Opened 5 years ago
Last modified 4 years ago
#24457 needs_work task
modernize sage.rings.real_mpfr
Reported by: | vdelecroix | Owned by: | |
---|---|---|---|
Priority: | major | Milestone: | sage-8.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 floating-point 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 floating-point 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 5 years ago by
- Branch #24456 deleted
- Dependencies set to #24456
comment:2 Changed 5 years ago by
comment:3 Changed 5 years ago by
- Description modified (diff)
comment:4 Changed 5 years ago by
- Description modified (diff)
comment:5 Changed 5 years ago by
- Branch set to u/rws/rename_sage_rings_real_mpfr_realfield
comment:6 Changed 5 years ago by
- Commit set to 71f9fd0b38f89ca57fa70f88bc80fa62d124322c
- Status changed from new to needs_review
comment:7 Changed 5 years ago by
Ah I forgot the string representation part.
comment:8 Changed 5 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 5 years ago by
- Branch set to u/rws/24457-1
comment:10 Changed 5 years ago by
- Commit set to 8bdf66b2d1e120977cb3b0c5a04bdbfcf988a465
- Status changed from needs_work to needs_review
comment:11 in reply to: ↑ description ; follow-up: ↓ 15 Changed 5 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 5 years ago by
- Status changed from needs_review to needs_work
comment:13 Changed 5 years ago by
- Branch changed from u/rws/24457-1 to u/rws/24457-2
comment:14 Changed 5 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 ; follow-up: ↓ 16 Changed 5 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 ; follow-ups: ↓ 17 ↓ 19 Changed 5 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 ; follow-up: ↓ 18 Changed 5 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 5 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 ; follow-up: ↓ 20 Changed 5 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 ; follow-up: ↓ 21 Changed 5 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 ; follow-up: ↓ 22 Changed 5 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 5 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 5 years ago by
- Branch changed from u/rws/24457-2 to u/rws/24457-3
comment:24 Changed 5 years ago by
- Branch changed from u/rws/24457-3 to u/rws/24457-4
comment:25 Changed 5 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 follow-up: ↓ 28 Changed 5 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 sage-devel ?
- Regarding the naming, i would personally prefer
Floating-point
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 5 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 5 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 sage-devel ?
- Regarding the naming, i would personally prefer
Floating-point
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 Floating-point.
- where is the doc ?
comment:29 Changed 5 years ago by
- Dependencies #24456 deleted
- Description modified (diff)
comment:30 Changed 5 years ago by
- Description modified (diff)
comment:31 Changed 5 years ago by
- Branch u/rws/24457-4 deleted
- Commit 9b2a5c2a38cf20b52dab663cecffc1009eb6ecbd deleted
comment:32 Changed 5 years ago by
- Description modified (diff)
- Summary changed from rename sage.rings.real_mpfr.RealField to modernize sage.rings.real_mpfr
comment:33 Changed 5 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 5 years ago by
- Description modified (diff)
comment:35 Changed 5 years ago by
- Dependencies set to #24511
- Description modified (diff)
comment:36 follow-up: ↓ 37 Changed 5 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 ; follow-up: ↓ 42 Changed 5 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 5 years ago by
- Description modified (diff)
Note that is_RealField(X)
then becomes isinstance(X, RealField_class)
.
comment:39 Changed 5 years ago by
- Description modified (diff)
comment:40 Changed 5 years ago by
- Description modified (diff)
- Type changed from enhancement to task
comment:41 Changed 5 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 5 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 5 years ago by
- Description modified (diff)
comment:44 Changed 5 years ago by
- Description modified (diff)
comment:45 Changed 5 years ago by
- Description modified (diff)
comment:46 Changed 5 years ago by
- Description modified (diff)
comment:47 Changed 5 years ago by
- Description modified (diff)
comment:48 Changed 4 years ago by
- Status changed from needs_work to needs_review
Rebased on the remaining dependency.
comment:50 Changed 4 years ago by
- Dependencies #24511 deleted
- Milestone changed from sage-8.2 to sage-8.3
comment:51 Changed 4 years ago by
- Milestone changed from sage-8.3 to sage-8.4
update milestone 8.3 -> 8.4
In
real_mpfr.pyx
do you mean to renamecpdef RealField(int prec=53,...)
orcdef class RealField_class
?