Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

RealSet: Use self.__class__.__base__ in methods when constructing a new RealSet #34277

Open
mkoeppe opened this issue Aug 4, 2022 · 25 comments

Comments

@mkoeppe
Copy link
Contributor

mkoeppe commented Aug 4, 2022

... instead of hardcoding RealSet. This is preparation for #34278.

(split out from #32170 - sagemath/sagetrac-mirror@develop...public/32170)

Depends on #32181

CC: @yuan-zhou @tscrim

Component: basic arithmetic

Author: Binshuai Wang, Matthias Koeppe, Yuan Zhou

Branch/Commit: u/yzh/realset__use_self___class___in_methods_when_constructing_a_new_realset @ eef3886

Issue created by migration from https://trac.sagemath.org/ticket/34277

@mkoeppe mkoeppe added this to the sage-9.7 milestone Aug 4, 2022
@mkoeppe
Copy link
Contributor Author

mkoeppe commented Aug 5, 2022

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Aug 5, 2022

Author: Binshuai Wang, ...

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Aug 5, 2022

comment:2

Cherry-picked from #32170


New commits:

5f60c49make a whole change for RealSet class with using 'self.__class__' and '@classmethod'

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Aug 5, 2022

Commit: 5f60c49

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Aug 5, 2022

Branch pushed to git repo; I updated commit sha1. Last 10 new commits:

44926c8better pyflakes, better handle of RealSet(x != oo)
c9d4746update RealSet.union and RealSet.intersection to handle several real sets
ebd3b77Better pygments style
73e5aa3Make white logo transparent to match with furo
523b49aMerge branch 'u/klee/34252' of git://trac.sagemath.org/sage into t/32181/realset__faster_operations_by_scan_line__merging__techniques
a864fdcfix typo
3f6a0acrevive RealSet.normalize
79f2014revise the docstring of RealSet.is_connected
1ef7dd1remove unnecessary bool()
a856e5dMerge #32181

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Aug 5, 2022

Changed commit from 5f60c49 to a856e5d

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Aug 5, 2022

Dependencies: #32181

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Aug 5, 2022

Changed author from Binshuai Wang, ... to Binshuai Wang, Matthias Koeppe, ...

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Aug 5, 2022

comment:4

Merged #32181 and resolved merge conflicts.

Haven't checked whether everything that needs to be changed to __class__ has been changed.

@mkoeppe

This comment has been minimized.

@yuan-zhou
Copy link

comment:6

Bug example:

sage: r = RealSet(2,10)
sage: RealSet(x > 2).intersection(RealSet(x < 10)) is RealSet(r[0], normalized=True)

Expect True. Got False.

@yuan-zhou
Copy link

comment:7

In the commit 5f60c49 from #32170,
why is it sometimes isinstance(other, RealSet) but isinstance(right, self.__class__) in other places? Why is it RealSet.point(...) and RealSet._prep(...) sometimes but self.__class__.real_line() in other places?

@yuan-zhou
Copy link

comment:8

In def union(self, *real_set_collection) and intersection(self, *real_set_collection),
why is it

intervals = tuple(self._scan_to_intervals(scan, lambda i: i > 0))

instead of self.__class__._scan_to_intervals?

(It was RealSet._scan_to_intervals)

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Aug 5, 2022

comment:9

Replying to @yuan-zhou:

In def union(self, *real_set_collection) and intersection(self, *real_set_collection),
why is it

intervals = tuple(self._scan_to_intervals(scan, lambda i: i > 0))

instead of self.__class__._scan_to_intervals?

(It was RealSet._scan_to_intervals)

_scan_to_intervals is a staticmethod, so self._scan_to_intervals(...) is the same thing as self.__class__._scan_to_intervals.

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Aug 5, 2022

comment:10

Replying to @yuan-zhou:

In the commit 5f60c49 from #32170,
why is it sometimes isinstance(other, RealSet) but isinstance(right, self.__class__) in other places? Why is it RealSet.point(...) and RealSet._prep(...) sometimes but self.__class__.real_line() in other places?

As I said, I haven't checked whether all necessary changes have been made

@yuan-zhou
Copy link

Changed commit from a856e5d to c97b9f7

@yuan-zhou
Copy link

comment:12

Unfortunately, the previous bug example remains.

sage: r = RealSet(2,10)
sage: r is r.__class__(r)
True
sage: r is r.__class__(r[0])
False
sage: r is r.__class__(r[0], normalized=True)
False

New commits:

c97b9f7change RealSet(...) constructor to self.__class__(...)

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Aug 5, 2022

Changed author from Binshuai Wang, Matthias Koeppe, ... to Binshuai Wang, Matthias Koeppe, Yuan Zhou

@mkoeppe mkoeppe changed the title RealSet: Use self.__class__ in methods when constructing a new RealSet RealSet: Use self.__class__.__base__ in methods when constructing a new RealSet Aug 6, 2022
@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Aug 6, 2022

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

42f4dd6Use self.__class__.__base__ in methods when constructing a new RealSet

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Aug 6, 2022

Changed commit from c97b9f7 to 42f4dd6

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Aug 7, 2022

comment:17

More changes in the same direction can be cherry-picked from #34278

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Aug 7, 2022

Changed commit from 42f4dd6 to eef3886

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Aug 7, 2022

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

c3893aasrc/sage/sets/real_set.py: Use self.METHOD, cls.METHOD instead of RealSet.METHOD; use super()
dab16c2src/sage/sets/real_set.py: Make normalize a @classmethod
0574efdsrc/sage/sets/real_set.py (RealSet_base.__init__): Check normalized
eef3886src/sage/sets/real_set.py: Use self.METHOD, cls.METHOD instead of RealSet.METHOD; use super() (fixup)

@yuan-zhou
Copy link

comment:19

cherry-picked the relevant commits from #34278.

@mkoeppe mkoeppe modified the milestones: sage-9.7, sage-9.8 Aug 31, 2022
@mkoeppe mkoeppe modified the milestones: sage-9.8, sage-9.9 Jan 7, 2023
@mkoeppe mkoeppe removed this from the sage-10.0 milestone Mar 16, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants