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.

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

[stdlib_io] disp(display your data) #445

Closed
wants to merge 19 commits into from

Conversation

zoziha
Copy link
Contributor

@zoziha zoziha commented Jun 25, 2021

This branch develops subroutines disp and format_string for stdlib based on the disp and num2str functions in the forlab library.
So, now disp API is:

call [[stdlib_io(module):disp(interface)]]( [value, header, unit, brief] )

disp subroutine

Easily display strings, scalars and rank-1/rank-2 arrays to the screen (or another output unit).

Effects
 disp(string):
 string
 It is a note.

 disp(r):
 [matrix size: 2×3]
 -0.1000E-10  -0.1000E+11    1.000
   1.000        1.000        1.000
 disp(c):
 [matrix size: 2×3]
            (1.000,0.000)             (1.000,0.000)             (1.000,0.000)
            (1.000,0.000) (-0.1000E+11,-0.1000E+11)             (1.000,0.000)
 disp(i):
 [matrix size: 2×3]
           1            1            1
           1            1            1
 disp(l):
 [matrix size: 10×10]
           T            T            T          ...            T
           T            T            T          ...            T
           T            T            T          ...            T
           :            :            :            :            :
           T            T            T          ...            T
 disp(c_3d, 3):
 [matrix size: 2×100]
            (2.000,0.000)             (2.000,0.000)             (2.000,0.000)                       ...             (2.000,0.000)  
            (2.000,0.000)             (2.000,0.000)             (2.000,0.000)                       ...             (2.000,0.000)  
 disp(c_3d, 2):
 [matrix size: 100×20]
            (2.000,0.000)             (2.000,0.000)             (2.000,0.000)                       ...             (2.000,0.000)  
            (2.000,0.000)             (2.000,0.000)             (2.000,0.000)                       ...             (2.000,0.000)  
            (2.000,0.000)             (2.000,0.000)             (2.000,0.000)                       ...             (2.000,0.000)  
                        :                         :                         :                         :                         :  
            (2.000,0.000)             (2.000,0.000)             (2.000,0.000)                       ...             (2.000,0.000) 

format_string function

(see #444)

Links

  1. (issue see Function to format variable as string #435 )🔰
  2. keurfonluu/Forlab
  3. zoziha/forlab
  4. Add complex number Descriptor j3-fortran/fortran_proposals#212

1. src/stdlib_io_disp.fypp
2. src/tests/string/test_io_disp.f90
3. stdlib_io.fypp%disp interface
modify:
1. doc/spec/stdlib_io.md%disp(doc)
2. src/Makefile.manual%disp(make)
3. src/tests/string/Makefile.manual%disp(make)
4. src/CMakelists.txt%disp(cmake)
5. src/CMakelists.txt%disp(cmake)
note:
make test passed.
cmake test passed
src/stdlib_io.fypp Outdated Show resolved Hide resolved
@awvwgk awvwgk added the reviewers needed This patch requires extra eyes label Jun 25, 2021
@awvwgk awvwgk self-requested a review June 25, 2021 19:32
@awvwgk
Copy link
Member

awvwgk commented Jun 25, 2021

Thanks @zoziha, @St-Maxwell for opening those pull requests. I think it is fine to have two PRs here, we will first discuss on #444 about the string formatting routines and once we agree on the API we will continue with the review of the display function. How does the plan sound to you?

src/stdlib_io_disp.fypp Outdated Show resolved Hide resolved
src/stdlib_io_disp.fypp Outdated Show resolved Hide resolved
src/stdlib_io.fypp Outdated Show resolved Hide resolved
src/stdlib_io.fypp Outdated Show resolved Hide resolved
src/stdlib_io_disp.fypp Outdated Show resolved Hide resolved
src/stdlib_io_disp.fypp Outdated Show resolved Hide resolved
src/stdlib_io_disp.fypp Outdated Show resolved Hide resolved
src/stdlib_io_disp.fypp Outdated Show resolved Hide resolved
src/stdlib_io_disp.fypp Outdated Show resolved Hide resolved
---
- stdlib_io.fypp
1. add disp interface comments
2. remove `impure` label
- stdlib_io_disp.fypp
1. remove `non_intrinsic` label
2. update some `format_string` to `to_string`
3. update `fmt_r` & `fmt_c`
4. add some routine comments
5. update the writing of some statements
- update test_io_disp.f90
- notes
1. make passed
2. cmake passed
Copy link
Contributor Author

@zoziha zoziha left a comment

Choose a reason for hiding this comment

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

Strange discovery

src/stdlib_io_disp.fypp Outdated Show resolved Hide resolved
Copy link
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 for this PR.
It might be good to submit 2 PRs, instead of one. It could help the reviewers.

doc/specs/stdlib_io.md Outdated Show resolved Hide resolved
doc/specs/stdlib_io.md Outdated Show resolved Hide resolved
doc/specs/stdlib_io.md Outdated Show resolved Hide resolved
doc/specs/stdlib_io.md Outdated Show resolved Hide resolved
doc/specs/stdlib_io.md Outdated Show resolved Hide resolved
src/stdlib_io.fypp Outdated Show resolved Hide resolved
src/stdlib_io_disp.fypp Outdated Show resolved Hide resolved
src/stdlib_io_disp.fypp Outdated Show resolved Hide resolved
src/stdlib_strings_format_string.fypp Outdated Show resolved Hide resolved
src/tests/io/test_io_disp.f90 Outdated Show resolved Hide resolved
src/stdlib_io.fypp Outdated Show resolved Hide resolved
Update `disp` docs;
Adopt `select case` block;
Adopt the `fypp` inline rank writing;
Rename `disp` input arg: val -> value.
@zoziha

This comment has been minimized.

@zoziha zoziha changed the title [feature] disp(display your data) & format_string(format other type to string) [feature] disp(display your data) & format_string(format other type to string, see #444) Jul 6, 2021
@St-Maxwell
Copy link
Member

I may have some questions about the scheme of setting output_unit:

  1. A single * Already means the default output location, which is the same as [iso_fortran_env(module):output_unit(integer)] and belongs to the standard output, or is it that I understand it wrong, they are different?
  2. Or your idea is to add a input argument for setting the output_unit number to the disp subroutine.
    If it is the latter, you will encounter such a problem, and the polymorphic interface with two optional arguments will encounter such a problem (see fortran interface optional). At this time, it is better to consider rewriting a set of functions disp(value [, string]) + disp(value, unit [, string]). (I can't express what I mean very well)

The existing test (comment) does not report an error if there is any issue, because it simply outputs data to the screen (output_unit) and cannot be tested well. If the output_unit scheme is improved, further testing may be possible.

I think the interface of disp would be something like disp(value [, unit=output_unit] [, string]). The dummy arguments unit and string are optional, and unit has a default value output_unit.
In fact, disp reminds me of raw_print function in Armadillo. That's why I proposed a similar interface for disp.

@arjenmarkus
Copy link
Member

arjenmarkus commented Jul 6, 2021 via email

@zoziha

This comment has been minimized.

@zoziha
Copy link
Contributor Author

zoziha commented Jul 7, 2021

Hello, I got a suggestion from @St-Maxwell and tried to implement it simply. I created a new branch disp_unit_brief in which I implemented optional unit and brief. If all of you feel good, I can merge it to this PR.

Example

(see stdlib_io.md/disp_example)

 disp(string):
 string
 It is a note.

 disp(r):
 [matrix size: 2×3]
 -0.1000E-10  -0.1000E+11    1.000
   1.000        1.000        1.000
 disp(c):
 [matrix size: 2×3]
            (1.000,0.000)             (1.000,0.000)             (1.000,0.000)
            (1.000,0.000) (-0.1000E+11,-0.1000E+11)             (1.000,0.000)
 disp(i):
 [matrix size: 2×3]
           1            1            1
           1            1            1
 disp(l):
 [matrix size: 10×10]
           T            T            T          ...            T
           T            T            T          ...            T
           T            T            T          ...            T
           :            :            :            :            :
           T            T            T          ...            T
 disp(c_3d, 3):
 [matrix size: 2×100]
            (2.000,0.000)             (2.000,0.000)             (2.000,0.000)                       ...             (2.000,0.000)  
            (2.000,0.000)             (2.000,0.000)             (2.000,0.000)                       ...             (2.000,0.000)  
 disp(c_3d, 2):
 [matrix size: 100×20]
            (2.000,0.000)             (2.000,0.000)             (2.000,0.000)                       ...             (2.000,0.000)  
            (2.000,0.000)             (2.000,0.000)             (2.000,0.000)                       ...             (2.000,0.000)  
            (2.000,0.000)             (2.000,0.000)             (2.000,0.000)                       ...             (2.000,0.000)  
                        :                         :                         :                         :                         :  
            (2.000,0.000)             (2.000,0.000)             (2.000,0.000)                       ...             (2.000,0.000) 

Reference links

@zoziha
Copy link
Contributor Author

zoziha commented Jul 10, 2021

I found two problems during my coding process:

  1. Other data types, such as integer, complex, character and logical types, are all right-aligned, but the default printing format of real types is neither left-aligned nor right-aligned. This looks weird in disp_brief, of course, we can ignore it, this problem is not fatal.
    image
  2. I saw stdlib set the derived string type string_type, I wonder if disp needs to support it?

Copy link
Contributor Author

@zoziha zoziha left a comment

Choose a reason for hiding this comment

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

Add support for string_type type initially, and ready to get your feedback and update docs.
This PR is for stdlib_io_disp routines, #444 PR is for stdlib_string_format_string routines.
Thank you all~

src/stdlib_io_disp.fypp Show resolved Hide resolved
src/stdlib_io_disp.fypp Outdated Show resolved Hide resolved
Merge branch 'zoziha/feature/disp' into feature/disp_deep_brief
@zoziha zoziha changed the title [feature] disp(display your data) & format_string(format other type to string, see #444) [feature] disp(display your data)~~& format_string(format other type to string, see #444)~~ Aug 23, 2021
@zoziha zoziha changed the title [feature] disp(display your data)~~& format_string(format other type to string, see #444)~~ [feature] disp(display your data) Aug 23, 2021
@zoziha zoziha changed the title [feature] disp(display your data) [stdlib_io] disp(display your data) Aug 23, 2021
@zoziha
Copy link
Contributor Author

zoziha commented Aug 23, 2021

Because to_string has been merged and disp depends on to_string, I updated disp this time.
Come to think of it, disp seems to have enough possibilities. We welcome your suggestions for extension and modification.
The purpose of disp is to help users easily know the matrix data and print statements.

call disp("It is a note.")
call disp(x, "I am a scalar:")
call disp(A(1:10, 2:4), "Let me see the array:")

@zoziha

This comment has been minimized.

@awvwgk awvwgk removed the reviewers needed This patch requires extra eyes label Sep 25, 2021
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.

5 participants