Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Single and quadruple precisions for load/savetxt #37

Merged
merged 2 commits into from
Dec 22, 2019
Merged

Single and quadruple precisions for load/savetxt #37

merged 2 commits into from
Dec 22, 2019

Conversation

jvdp1
Copy link
Member

@jvdp1 jvdp1 commented Dec 22, 2019

No description provided.

certik
certik requested changes Dec 22, 2019
View reviewed changes
Copy link
Member

@certik certik left a comment

Choose a reason for hiding this comment

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

I left a few minor comments. I think this looks great. Thank you!

src/stdlib_experimental_io.f90 Outdated
end interface

contains

!PUBLIC
Copy link
Member

Choose a reason for hiding this comment

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

What does this !PUBLIC do?

Copy link
Member Author

Choose a reason for hiding this comment

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

I am used to sort the different procedures in groups of public and private procedures. This !PUBLIC (and !PRIVATE below) refer to these two groups. I can remove them.

Copy link
Member Author

Choose a reason for hiding this comment

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

Now removed.

Copy link
Member

Choose a reason for hiding this comment

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

I see. Once we fix #4, we'll consolidate all such things.

src/stdlib_experimental_io.f90 Outdated
@@ -1,28 +1,65 @@
module stdlib_experimental_io
use iso_fortran_env, only: sp=>real32, dp=>real64
use iso_fortran_env, only: sp=>real32, dp=>real64 ,qp=>real128
Copy link
Member

Choose a reason for hiding this comment

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

Space after , and no space before?

Copy link
Member Author

Choose a reason for hiding this comment

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

Corrected

src/stdlib_experimental_io.f90 Show resolved Hide resolved
src/stdlib_experimental_io.f90 Outdated
logical :: lastwhite
real(dp) :: r
integer :: s
integer ::nrow,ncol,i
Copy link
Member

Choose a reason for hiding this comment

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

Space after :: ?

Copy link
Member Author

Choose a reason for hiding this comment

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

Corrected

src/stdlib_experimental_io.f90 Outdated
end do

rewind(s)
ncol=number_of_columns(s)
Copy link
Member

Choose a reason for hiding this comment

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

Space around = ?

Copy link
Member Author

Choose a reason for hiding this comment

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

Corrected

src/stdlib_experimental_io.f90 Outdated
end subroutine


!PRIVATE
Copy link
Member

Choose a reason for hiding this comment

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

I assume this does a similar thing as the !PUBLIC above.

Copy link
Member Author

Choose a reason for hiding this comment

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

Removed

certik
certik approved these changes Dec 22, 2019
View reviewed changes
Copy link
Member

@certik certik left a comment

Choose a reason for hiding this comment

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

+1 to merge!

@certik
Copy link
Member

certik commented Dec 22, 2019

This is +1 from me to merge as is. The changes are straightforward, they do not change the public API and they all happen in experimental, so I am going to merge it.

@certik certik merged commit 5186134 into fortran-lang:master Dec 22, 2019
@certik
Copy link
Member

certik commented Dec 22, 2019

@jvdp1 thanks for the contribution!

@jvdp1 jvdp1 deleted the loadtxt2 branch December 23, 2019 10:07
@fiolj
Copy link
Contributor

fiolj commented Feb 2, 2020 •
edited
Loading

I've mentioned in the issue on quadrature that I'd like to have implementations for integrating complex functions. I think, that seeing as complex numbers are implemented in fortran, we could include them in the library. I've made the modifications to common.fypp, stdlib_experimental_io.fypp, and test_savetxt.f90 to add support to loadtxt and savetxt for complex numbers. Is that desired? Should I open a pull request?

@jvdp1
Copy link
Member Author

jvdp1 commented Feb 2, 2020

@fiolj same comment as in #62 comment: I think it is a good idea to add complex numbers to the library. Because it is an addition to already existing API, I suggest to open a PR.
Could you also add them to sdtlib_experimental_stat, please? So the whole library would support complex numbers. The community can then further discuss its utility/wish around a PR.

@fiolj
Copy link
Contributor

fiolj commented Feb 2, 2020

Yes @jvdp1, I'll try this week to put in stats, probably not before tuesday or wednesday. When I was to do it I realized that I should start with io
I just put the pull request to stdlib_experimental_io, and modified optval, I'll put this also in stats.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@certik certik certik approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@jvdp1 @certik @fiolj