Skip to content

Ticket 1236 suggested fix. #127

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

Closed
wants to merge 4 commits into from
Closed

Ticket 1236 suggested fix. #127

wants to merge 4 commits into from

Conversation

pletnes
Copy link
Contributor

@pletnes pletnes commented Jul 31, 2011

Ticket 1236 suggested fix. I suggest giving a separate keyword argument for structured arrays - it may be nice to be able to add a manual header, too

pletnes added 2 commits July 31, 2011 21:04
…nt for structured arrays - it may be nice to be able to add a manual header, too
@pletnes
Copy link
Contributor Author

pletnes commented Aug 14, 2011

How do you like those changes? I wrote some tests as well as made the default commentstr='# ' which loadtxt should recognize automagically.

@@ -853,6 +854,13 @@ def savetxt(fname, X, fmt='%.18e', delimiter=' ', newline='\n'):
Character separating columns.
newline : str
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All the variables with default values need "optional" appended to the type like so:

newline : str, optional

@charris
Copy link
Member

charris commented Aug 15, 2011

Looking good, the tests help. I'm trying to think of a better name than 'commentstr', but not having much luck ;)

footer : str
String that will be written to the end of the file.
commentstr : str
String that will be prepended to the ``header`` and ``footer`` strings.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should expand this a bit for clarity, say by adding "...to mark them as comment lines.."

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changing "commentstr" to "comments" to make it consistent with loadtxt documentation.

…to "comments", to be in line with loadtxt documentation. cleaned up some documentation. Added code to deal with the possibility of header and footer containing newlines, where the "comments" string will be prepended to each line.
@charris
Copy link
Member

charris commented Aug 15, 2011

Looks good. Needs the ".. versionadded:: 2.0.0" bit for the new parameters. I apologize for not noticing that earlier. The '2.0.0' is probably going to be wrong, but we can grep and change it when the time comes for the next release.

@pletnes
Copy link
Contributor Author

pletnes commented Aug 15, 2011

I saw that and didn't want to add something in case I was wrong about the numbers. I'll put it in.

On 15. aug. 2011, at 16.42, charris wrote:

Looks good. Needs the ".. versionadded:: 2.0.0" bit for the new parameters. I apologize for not noticing that earlier. The '2.0.0' is probably going to be wrong, but we can grep and change it when the time comes for the next release.

Reply to this email directly or view it on GitHub:
#127 (comment)

@charris
Copy link
Member

charris commented Aug 16, 2011

OK. I'm not convinced having a separate comment string is much easier than just having the user prefix their own string, but I suppose if one is exporting in various formats it could be helpful. OTOH, it isn't like having it is a big complication.

Pushed 730b861..5cf0a07.

@charris charris closed this Aug 16, 2011
@pletnes
Copy link
Contributor Author

pletnes commented Aug 16, 2011

I ws thinking something like converting a file from a measurement setup, or a simulation, where you may have a long autogenerated string and it will be slightly more messy to insert comment characters at the correct places. Here savetxt will do it for you. Not that it is a huge issue either way...

On 16. aug. 2011, at 03:44, charrisreply@reply.github.com wrote:

OK. I'm not convinced having a separate comment string is much easier than just having the user prefix their own string, but I suppose if one is exporting in various formats it could be helpful. OTOH, it isn't like having it is a big complication.

Pushed 730b861..5cf0a07.

Reply to this email directly or view it on GitHub:
#127 (comment)

luyahan pushed a commit to plctlab/numpy that referenced this pull request Apr 25, 2024
test: Add testing tools and debugging tools
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants