-
-
Notifications
You must be signed in to change notification settings - Fork 528
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
Customize printing of real numbers #7682
Comments
comment:1
Note that these extra zeros were introduced in the printing by the switch to pynac. They were eliminated a while ago in maxima by #4572. |
comment:2
I'm declaring a total feature freeze on sage-4.3. |
comment:3
Here is a first cut of a print_options model for RealField that lets you specify default printing options for any RealField, all RealFields, and still lets you override the defaults when you actually print the numbers. It mostly works. Docs need to be updated, deprecation warnings need to be added, and the interface for scientific notation needs to be revamped (no_sci and sci_not are confusing and inconsistent keyword choices!). |
Author: Jason Grout |
comment:6
I refreshed the patch which a much more comprehensive patch. This patch fixes all doctests in rings/*.pyx (running long doctests now). Documentation still needs to be updated, though, and some new doctests to test the new functionality. |
comment:7
Okay, patch passes all (non-long) doctests in Sage. |
comment:8
Usually rings are supposed to be immutable, but I think this general idea is worth it. As for the interface, setting attributes on RR directly isn't very consistent with the way we do things in Sage--it's probably be better to have a RR.print_options(...) that takes keywords. (This way it could have a nice docstring as well.) |
comment:9
Thanks for the style review. Okay, I can change print_options to a function, though I don't think it's "better", maybe just more consistent and slightly worse (i.e., unpythonic and more typing) On another note, I just changed the latexing of a real field to indicate the precision and rounding, so that RealField(100,rnd='RNDD') prints as \Bold{R}^{-}_{100}. The subscript is the precision, the superscript is '+' for RNDU, '-' for RNDD, 0 for RNDZ, and nothing for RNDN. What do you think? It mirrors better the textual description of the field that indicates precision and rounding. |
comment:10
As for immutability; what if we don't consider the printing options as part of the ring (i.e., it's not part of the hash for the cache, like it is now)? Then a user can temporarily change the ring's printing operations, but none of the printing options are considered in testing for equality, stored in pickles, etc. |
comment:11
Replying to @robertwb:
Yes, but how do you get the value of a specific option (as opposed to setting it). |
comment:12
Replying to @robertwb:
The Cython "property" construct can also take a nice docstring. Is anything ever done with this docstring? |
Attachment: trac-7682-realfield-printing-options.patch.gz |
comment:13
Okay, this is getting big now. I went through real_mpfr.pyx and added a lot of doctests and documentation. There are four doctests failing right now because I'm not sure if they are bugs or if they are right. Here they are:
Are those four answers right (the "Got:" parts)? See #8074 for more corner cases. |
comment:14
I rebased this patch to apply to 4.3.3 (it conflicted with a big rewrite by was and a small patch by robertwb). The patch touches a lot of areas (and adds lots of doctests!), so a quick review would help avoid having to rebase it again. |
Attachment: trac-7682-realfield-printing-options.2.patch.gz apply instead of previous patch. Rebased to Sage 4.3.3 |
comment:15
I uploaded a new patch which corrects several doctests; all doctests in Sage now seem to pass with this patch applied. |
comment:22
#9261 asks for one of the features implemented in this patch (setting the printing style for real interval fields). |
comment:23
After experimenting for a few minutes, it looks like something like #9261 is almost implemented in this patch, but not quite. |
apply on top of previous patch |
comment:24
Attachment: trac-7682-realintervalfield-printing.patch.gz I added a patch which implements the feature wanted in #9261:
Doctests and documentation still needs to be written for this. And maybe convenience functions for setting these two options (i.e., make the syntax in #9261 work). |
comment:25
Jason, sorry, I was not aware of this ticket. I see you have invested a lot of time in it. However I am About reducing or increasing the number of printed zeroes with respect to the internal precision,
In addition I don't understand how you achieve this:
What happens with Also, what happens with numbers with tiny or huge exponent, say Just my 2 cents. Paul PS: however, the patch for #9261 looks very nice. Can't you make it independent of that ticket? |
comment:26
Replying to @zimmermann6:
I agree. That's the default in Sage now, though (and led to this patch, as it was hiding too much in my numerical analysis class!) So changing it should probably be a different ticket, and after this patch, should just be a one liner change to the defaults.
Good questions. It's been a while since I worked with this patch (other than the rough patch from yesterday). I'll try to see what changes are changes I made, as opposed to what things were already in Sage. The things that were already in Sage can be dealt with on another ticket.
Yes, though it's easier to build on top of the framework here, and I hope better in the long run. |
comment:27
Applying these patches to 4.4.2 gives several doctest errors like this:
|
comment:28
It appears that the problem above occurs with just the last patch. |
This comment has been minimized.
This comment has been minimized.
comment:29
Paul: I think I understand your comment now. I did not implement the original suggestion, but instead provided a framework for field-wide printing options and just wrapped what Sage currently provides. You bring up some good questions about the design which are out of scope for the current patch attached. Given this, I think it would be best to either change the scope of the ticket to reflect what the patch actually does, or make a new ticket for the patch and keep this ticket around for a design discussion of how (or whether it is desirable!) to implement the features described in the description. |
This comment has been minimized.
This comment has been minimized.
comment:31
Replying to @jasongrout:
I would prefer this. Paul |
comment:32
I actually dislike the goal of this patch: I don't think it's a good idea to have Somebody wants to know what 128 bits of
Then, days later (but in the same Sage session) they're playing around with the internals of AA/QQbar:
Why is that last number printed in scientific notation? Oh yes, it's because we changed RR128 days ago. I realize that you're just extending a design that's been in Sage for years (since before I started working on Sage), but IMHO it's a bad design, and this just makes it worse. I can think of two ways to fix this:
My vote would be for option 1, but I could live with either option. |
comment:33
Replying to @sagetrac-cwitty:
I agree. Another reason to add to your argument above is that Sage does coercing between different realfield precisions, so you might have a number that is printed one way, then Sage automatically coerces to a different precision for an operation and your result is printed a different way. I think (1) is a better option, given the caching strategy used. For my use-case (teaching numerical analysis), option (1) is better than the patch on this ticket. So do you propose eliminating the sci_not options to RealField? Do you propose eliminating the arguments to the str function? Note that I think your suggestion will be relatively straightforward to accommodate on this patch, since the patch defines module-level defaults. We should be able to just remove the code that overrides the module-level defaults and stores a user value. Note that this patch also unifies several different options for scientific notation that were scattered in different places in the code, so I think it is better to build (or cut things out) on this patch rather than throw it away altogether. |
comment:34
So do you propose eliminating the sci_not options to RealField?? Do you propose eliminating the arguments to the str function? Yes, my vote would be to eliminate sci_not in
Further comments: I haven't really reviewed the actual patch, but I did just notice that the new docstring for .str() has no doctests for no_sci. I think it should end with something like:
followed by the tests for no_sci that used to be there (assuming there were some, I haven't actually checked). |
comment:35
ping (?) |
comment:36
pong. I would love to see this ticket resolved, but as you can see, there is another major rewrite needed before it is ready. |
comment:42
How lovely to revive an 8 year old ticket ... In the mean time, python has grown a new string formatting method. If we implement a
No need to fuss with global state ... if people want more control over the typesetting of their floats, they can just use the standard python tools (or the tools already available on It might be a nice beginner's exercise to write the appropriate |
comment:44
I wanted to have fun with I expected Sage would print me
is there a way to print |
comment:45
Replying to @sheerluck:
A solution is:
|
comment:46
Replying to @egourgoulhon:
Thank you! |
From http://groups.google.com/group/sage-support/browse_thread/thread/06756df51d828bf4
Additionally, and more flexibly, we could just have something like:
or more pythonically
Edit: the patches below do not allow you to set a C format string or number of digits, but they do provide a framework for setting field-wide printing options, and wrap the current printing options Sage has implemented (plus a few minor extensions, I think).
CC: @robertwb @sagetrac-jkantor @williamstein @kcrisman @egourgoulhon
Component: numerical
Author: Jason Grout
Issue created by migration from https://trac.sagemath.org/ticket/7682
The text was updated successfully, but these errors were encountered: