Skip to content

Savetxt unit#1085

Closed
fiolj wants to merge 142 commits intofortran-lang:masterfrom
fiolj:savetxt-unit
Closed

Savetxt unit#1085
fiolj wants to merge 142 commits intofortran-lang:masterfrom
fiolj:savetxt-unit

Conversation

@fiolj
Copy link
Copy Markdown
Contributor

@fiolj fiolj commented Jan 5, 2026

This PR aims to add optional arguments to savetxt, that behave similar to numpy's savetxt.

This is associated with Issue 263 and this discussion thread.

It add the possibility of supplying the unit of an open file instead of a filename (which could be used for output_unit for instance)

This implementation is quite simple. The main changes are:

  1. Add optional header and footer (that could be commented out with a character, currently defaults to '#')
  2. Changes delimiter to an arbitrary length string. I am not completely sure that is really useful. Initially I've added this because np.savetxt (Numpy's) allows character or arbitrary strings but np.loadtxt requires it to be a length 1 char.
  3. Add the possibility to override default format for saving data

@codecov
Copy link
Copy Markdown

codecov bot commented Jan 5, 2026

Codecov Report

❌ Patch coverage is 81.30719% with 143 lines in your changes missing coverage. Please review.
✅ Project coverage is 68.76%. Comparing base (fb63d7e) to head (e4f0541).
⚠️ Report is 57 commits behind head on master.

Files with missing lines Patch % Lines
src/datetime/stdlib_datetime.f90 75.41% 74 Missing ⚠️
example/datetime/example_datetime_usage.f90 0.00% 46 Missing ⚠️
example/io/example_savetxt.f90 0.00% 13 Missing ⚠️
...pecialmatrices/example_sym_tridiagonal_dp_type.f90 0.00% 7 Missing ⚠️
test/datetime/test_datetime.f90 99.05% 3 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #1085      +/-   ##
==========================================
+ Coverage   68.00%   68.76%   +0.75%     
==========================================
  Files         404      408       +4     
  Lines       12935    13697     +762     
  Branches     1392     1543     +151     
==========================================
+ Hits         8797     9419     +622     
- Misses       4138     4278     +140     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@jalvesz
Copy link
Copy Markdown
Contributor

jalvesz commented Jan 6, 2026

Thanks for this @fiolj. Would you mind reverting the changes not related to the implementation? (styling) There are too many and it renders difficult to read through the PR. You can check the style_guide for info https://github.com/fortran-lang/stdlib/blob/master/STYLE_GUIDE.md

One thing, white spaces in-between parentheses and an intrinsic function are not recommended ( open (...), allocate (...), etc )

@jalvesz jalvesz linked an issue Jan 6, 2026 that may be closed by this pull request
@fiolj
Copy link
Copy Markdown
Contributor Author

fiolj commented Jan 6, 2026

Thanks @jalvesz, I've fixed the formatting

Copy link
Copy Markdown
Member

@jvdp1 jvdp1 left a comment

Choose a reason for hiding this comment

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

Thank you @fiolj . Here are some suggestions

@fiolj
Copy link
Copy Markdown
Contributor Author

fiolj commented Jan 15, 2026

Reviewing the arguments of savetxt I've rechecked and Numpy's has the following order:
np.savetxt(fname, X, fmt, delimiter, newline, header, footer, comments, encoding)

Currently, we have the order of fmt and delimiter inverted. Should we make it as Numpy or should we keep it?

Juan Fiol and others added 21 commits April 6, 2026 14:55
Co-authored-by: Jeremie Vandenplas <jeremie.vandenplas@gmail.com>
Co-authored-by: Jeremie Vandenplas <jeremie.vandenplas@gmail.com>
Added:
- comments with intent of variable `fout`
- stop the program if unit file is not open
- clean-up comments
Added clarification of use with filename and unit.
Also added an example
Co-authored-by: José Alves <102541118+jalvesz@users.noreply.github.com>
Co-authored-by: José Alves <102541118+jalvesz@users.noreply.github.com>
Co-authored-by: José Alves <102541118+jalvesz@users.noreply.github.com>
@fiolj
Copy link
Copy Markdown
Contributor Author

fiolj commented Apr 6, 2026

Some check failed, should I do anything before it can be merged? I'm asking because the error seems unrelated to this branch

Could you rebase your branch, please? This issue should have been solved in a merged PR.

Done!

@jvdp1
Copy link
Copy Markdown
Member

jvdp1 commented Apr 6, 2026

hmmmm, I am puzzled. Why are all the changes in previously merged PRs present in Files changes?

@fiolj
Copy link
Copy Markdown
Contributor Author

fiolj commented Apr 7, 2026

hmmmm, I am puzzled. Why are all the changes in previously merged PRs present in Files changes?

Yes, I was too. It happened when I did the rebase (I usually just do a merge of the master).
The rebase started to replay several previous commits. I just went through them manually and made sure that the last version was equal to the one I already had (except for some unintentional white spaces that included my editor)

@jvdp1
Copy link
Copy Markdown
Member

jvdp1 commented Apr 7, 2026

@fiolj Strange. I usually merge the branches, too. Maybe to keep things clean, could you create a new branch from the last commit of this branch prior rebasing it, merge it with upstream/master and than open a new PR?

@fiolj fiolj mentioned this pull request Apr 7, 2026
@fiolj
Copy link
Copy Markdown
Contributor Author

fiolj commented Apr 7, 2026

@jvdp1 Done! PR #1177

@jvdp1
Copy link
Copy Markdown
Member

jvdp1 commented Apr 7, 2026

Closed as #1177 has been merged

@jvdp1 jvdp1 closed this Apr 7, 2026
@fiolj fiolj deleted the savetxt-unit branch April 8, 2026 00:10
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.

Add optional arguments to savetxt

9 participants