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:  sage8.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 followup: ↓ 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 followup: ↓ 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 followup: ↓ 9 Changed 3 years ago by
 Reviewers set to Ralf Stephan
Could you please add direct doctests to the member functions?
comment:8 followup: ↓ 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 followup: ↓ 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 reupload, but won't reptestlong
before late tonight (I *have* to go now...).
comment:17 followup: ↓ 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 ; followup: ↓ 19 Changed 3 years ago by
comment:19 in reply to: ↑ 18 ; followup: ↓ 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