Opened 3 years ago
Closed 3 years ago
#24119 closed enhancement (fixed)
Create an assuming() context manager
Reported by: | charpent | Owned by: | |
---|---|---|---|
Priority: | major | Milestone: | sage-8.1 |
Component: | symbolics | Keywords: | |
Cc: | rws | Merged in: | |
Authors: | Emmanuel Charpentier | Reviewers: | Ralf Stephan |
Report Upstream: | N/A | Work issues: | |
Branch: | 017f67c (Commits, GitHub, GitLab) | Commit: | 017f67c66da8aadfaefebb976528abacef6b58ad |
Dependencies: | Stopgaps: |
Description
Inspiration : Ralf Stephan in #22322. Thanks to him for nudging me in the right direction.
Goal : create a tool allowing to create reusable, stackable, assumption elements, thus allowing to get the functionality of Mathematica's Assuming[...,???]
What I mean :
sage: solve(x^2==4, x) [x == -2, x == 2] sage: with assuming(x>0): solve(x^2==4, x) [x == 2] sage: assumptions() []
Change History (21)
comment:1 Changed 3 years ago by
- Branch set to u/charpent/create_an_assuming___context_manager
comment:2 Changed 3 years ago by
- Commit set to 4b5f2ea72349f5caffa596762d2a833be6e5e121
- Status changed from new to needs_review
comment:3 follow-up: ↓ 6 Changed 3 years ago by
- Status changed from needs_review to needs_work
Aaarghh... I've found a subtle bug: after with assuming(): do_something()
, the original assumption database is destroyed.
That's due to the fact that forget()
is equivalent to forget(assumptions())
, i. e. forgets all facts in the database. Hoisted by a "lazy" facility...
I'll fix that (and add a couple doctests for exceptions.
Should we reconsider forget() <==> forget(assumptions())
?
comment:4 follow-up: ↓ 5 Changed 3 years ago by
- Commit changed from 4b5f2ea72349f5caffa596762d2a833be6e5e121 to c50defe06a74c6ead95ed63bb319eb549aed1d15
Branch pushed to git repo; I updated commit sha1. New commits:
c50defe | 24119 : Debugging, better doctests, a couple of typos
|
comment:5 in reply to: ↑ 4 Changed 3 years ago by
- Status changed from needs_work to needs_review
comment:6 in reply to: ↑ 3 Changed 3 years ago by
Replying to charpent:
Aaarghh... I've found a subtle bug: after
with assuming(): do_something()
, the original assumption database is destroyed.
[ Snip ... ]
To be absolutely clear : the problem existed when one used assuming
with an empty argument list : you wouldn't do that from the console, but this argument list might result from another computation, which might return an empty list.
Anyway, problem fixed.
comment:7 follow-up: ↓ 9 Changed 3 years ago by
- Reviewers set to Ralf Stephan
Could you please add direct doctests to the member functions?
comment:8 follow-up: ↓ 10 Changed 3 years ago by
Also add a doctest with the class that checks correctness with at least doubly stacked assumptions.
comment:9 in reply to: ↑ 7 Changed 3 years ago by
Replying to rws:
Could you please add direct doctests to the member functions?
What do you mean ? I added exactly ONE function (assuming()
), which I documented and doctested, and added a small blurb at the end of the module documentation. Since assuming()
is designed to interact exclusively with assume()
, I don't see what you mean.
comment:10 in reply to: ↑ 8 Changed 3 years ago by
Replying to rws:
Also add a doctest with the class that checks correctness with at least doubly stacked assumptions.
This I understand and will do. Stay tuned...
comment:11 Changed 3 years ago by
The class assuming
has three member functions, no? Check class hold_class
in #10035 where __enter__
and __exit__
is doctested.
comment:12 Changed 3 years ago by
- Commit changed from c50defe06a74c6ead95ed63bb319eb549aed1d15 to a43a32aa5a6b4627aef6067c5e606f20206a0c8a
Branch pushed to git repo; I updated commit sha1. New commits:
a43a32a | 24119 : doctest for stacks of assumptions.
|
comment:13 follow-up: ↓ 14 Changed 3 years ago by
- Commit changed from a43a32aa5a6b4627aef6067c5e606f20206a0c8a to 127d507348ca429602585bf0734e2741d889b135
Branch pushed to git repo; I updated commit sha1. New commits:
127d507 | 24119 : doctesting individual assuming's member functions.
|
comment:14 in reply to: ↑ 13 Changed 3 years ago by
comment:15 Changed 3 years ago by
I like it. Here some minor typos: double space after declaration
; :func:assume())
should have backticks; typo in useable in a in a
with
statement
; small caps begin new sentence in ...can be stacked. we can use this functionality can be used to...
also use is used twice; please correct (and by yhe way,
; finally please add an empty line before def __enter__
and before def __exit__
.
comment:16 Changed 3 years ago by
Passes ptestlong
with no error whatsoever.
I'll fix the typos and re-upload, but won't re-ptestlong
before late tonight (I *have* to go now...).
comment:17 follow-up: ↓ 18 Changed 3 years ago by
- Commit changed from 127d507348ca429602585bf0734e2741d889b135 to 017f67c66da8aadfaefebb976528abacef6b58ad
Branch pushed to git repo; I updated commit sha1. New commits:
017f67c | 24119 : small typos fixed.
|
comment:18 in reply to: ↑ 17 ; follow-up: ↓ 19 Changed 3 years ago by
comment:19 in reply to: ↑ 18 ; follow-up: ↓ 20 Changed 3 years ago by
Replying to charpent:
Replying to git:
Branch pushed to git repo; I updated commit sha1. New commits:
017f67c 24119 : small typos fixed.
Typos fixed, except a double space after
declaration
, which I didn't find. Builds and doctests OK.ptestlong
later tonight. Leaving NOW.
[ Later... ]
- Passes
ptestlong
with no error whatsoever. - Most of the 17 patchbots seem to like it, but all but one of them complain that "a plugin failed" ; it seems that this is the "coverage" plugin. Not my watch, I'm afraid...
- Four patchbots got a failure or a timeout :
- timeout in
src/sage/interfaces/giac.py
: doesn't seem related to my ticket ; - one failure on
src/sage/doctest/external.py
: ditto ; - timeout in
src/sage/graphs/graph_generators.py
: ditto ; - one failure in
src/sage/schemes/elliptic_curves/heegner.py
and one timeout insrc/sage/coding/linear_code.py
: ditto.
- timeout in
These failures seems "too random" to have a systematic cause, and may result from a race condition somewhere in the test infrastructure. We have been plagued with such "transient" failures for some time now.
Seems good to review...
comment:20 in reply to: ↑ 19 Changed 3 years ago by
- Status changed from needs_review to positive_review
Replying to charpent:
- Most of the 17 patchbots seem to like it, but all but one of them complain that "a plugin failed" ; it seems that this is the "coverage" plugin. Not my watch, I'm afraid...
The coverage plugin (of patchbot) looks if you added code without doctest. It uses the call sage --coverage
which you can do too. You fixed it however when I asked you so the recent patchbot runs no longer show it. It's all fine now.
comment:21 Changed 3 years ago by
- Branch changed from u/charpent/create_an_assuming___context_manager to 017f67c66da8aadfaefebb976528abacef6b58ad
- Resolution set to fixed
- Status changed from positive_review to closed
Initial proposition.
On 8.1beta9 + #24026+ #24006 + #23990 + #23923 + #24088 + #10035, passes
ptestlong
with no error whatsoever (1 timeout once onsage -t --long src/sage/modular/hecke/submodule.py
, unreproductible).==>
needs_review