-
-
Notifications
You must be signed in to change notification settings - Fork 10.8k
Ticket 1573 - suggested fix #136
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
Conversation
…fmt specifier, a list of fmt specifiers, and a string of fmt specifiers, for complex numbers in savetxt.
…plex numbers forgotten for fmt being a list or tuple
If anyone is wondering as to the 3 commits: I could not immediately agree with myself on exactly how the formatting rules should be... |
# `fmt` can be a string with multiple insertion points or a | ||
# list of formats. E.g. '%10.5f\t%10d' or ('%10.5f', '$10d') | ||
# Determine first if X is complex, where twice as many |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Shouldn't that be determine if the format is for complex?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If I hit savetxt(a_complex_array) or savetxt(a_real_array), savetxt should do the necessary magic, IMHO. And, how do you know that the format "is for complex"? I am unaware of any complex format specifier (but please, enlighten me if I'm wrong here).
On 13. aug. 2011, at 17.23, charris wrote:
# `fmt` can be a string with multiple insertion points or a # list of formats. E.g. '%10.5f\t%10d' or ('%10.5f', '$10d')
# Determine first if X is complex, where twice as many
Shouldn't that be determine if the format is for complex?
Reply to this email directly or view it on GitHub:
https://github.com/numpy/numpy/pull/136/files#r91202
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Well, the comment came after the check if 'X' was complex so I assumed you were checking for something else.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I forgot to remove that line. It's been committed now.
This looks good to me but it needs some tests. I really hate the capitol 'X' variable, but that seems to be used everywhere in npyio ;) |
The capital 'X' was there from before, but should be easy to replace with a search and replace. Where would I put the tests for this? I'll gladly write them if people feel the interface is the expected one - no need to test code if it will be rewritten before it is released. On 13. aug. 2011, at 17.27, charris wrote:
|
Don't worry about the 'X'. If you want to redo things at some point, make it a separate pull request. |
If you want more discussion of your choices, you should post to the list, maybe with some examples. |
I posted on the mailing list but saw no replies. On 13. aug. 2011, at 19:08, charrisreply@reply.github.com wrote:
|
I inclined to push this as no one else has weighed in. However, the documentation needs an update and the new functionality needs a test. |
I have been meaning to look at that, I just haven't had the time. On 26. aug. 2011, at 03:28, charrisreply@reply.github.com wrote:
|
Conflicts: numpy/lib/npyio.py Resolved conflicts and ran test suite.
I'll open a new pull request. |
feat: Add vsra_n_s8
Suggested fix, as advertised.
Do people like the form of the functionality? As of now, the fmt= kwarg kan be:
a) a single specifier, fmt='%.4e', resulting in numbers formatted like ' (%s+%sj)' % (fmt, fmt)
b) a full string specifying every real and imaginary part, e.g. '%.4e %+.4j' * 3 for 3 columns
c) a list of specifiers, one per column - in this case, the real and imaginary part must have separate specifiers, e.g. ['%.3e + %.3ej', '(%.15e%+.15ej)']
Should a test of this functionality be written?