String list

[arjenmarkus]

Here is a very preliminary implementation of a modules for handling lists of strings (see issue #268). The implementation is based on a derived type that holds an array of separate strings, but the suggestion made by @esterjo of using a single long string and an indexing array is well worth considering. Therefore consider this as a proposal for the behaviour and the interface.

[jvdp1]

Nice progress. Here are a few minor comments.
It seems that the position of the elements starts at 0 until n-1? Did I understand it correctly?

[milancurcic]

I agree with @jvdp1 regarding t_string -> string_type and t_stringlist -> stringlist_type.

The module should be called stdlib_stringlist (like the source file).

Would stringarray not be a more appropriate name for this derived type?

[arjenmarkus]

I have no problem with the suggestions :). Just one remark: "initialise" is quite acceptable according to a dictionary I checked (even though it is UK spelling). And I see why there are build failures: no documentation yet. Well, I will work on that. Op ma 21 dec. 2020 om 03:52 schreef Milan Curcic <notifications@github.com>:

…

I agree with @jvdp1 <https://github.com/jvdp1> regarding t_string -> string_type and t_stringlist -> stringlist_type. The module should be called stdlib_stringlist (like the source file). Would stringarray not be a more appropriate name for this derived type? — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#269 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAN6YR5L75ENZZBGSGLQMRTSV2Z6VANCNFSM4VBNHQ2Q> .

[jvdp1]

And I see why there are build failures: no documentation yet. Well, I will work on that.

It is partly due to that. There is also the issue that gfortran is called now gfortran-10 in MacOS. This PR #270 solves this issue. So you might want to rebase your branch to pass this action.

[milancurcic]

Just one remark: "initialise" is quite acceptable according to a dictionary I checked (even though it is UK spelling).

It is, it's why I scolded the ReviewDog. :) I think we can just ignore it.

[arjenmarkus]

Yes, my remark also reflects my personal grievances against those smart aleck spelling checkings. Op ma 21 dec. 2020 om 17:25 schreef Milan Curcic <notifications@github.com>:

…

Just one remark: "initialise" is quite acceptable according to a dictionary I checked (even though it is UK spelling). It is, it's why I scolded the ReviewDog. :) I think we can just ignore it. — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#269 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAN6YR75RDQ5KOIE2JFSSA3SV5ZGJANCNFSM4VBNHQ2Q> .

[jvdp1]

First quick comment: shouldn't a final procedure (that potential calls the method destroy) be present?

[arjenmarkus]

It could actually be a dummy - allocatables get cleaned up automatically.

[awvwgk]

I looked over the API documentations and left a few comments on the API design. Mainly the special positions list_head and list_end are inconsistent in my opinion.

[awvwgk]

This feels inconsistent to me. list_head represents an element outside of the list, while list_end represents an element in the list. I would prefer either having both special indices refer to elements inside or outside the list, but not mixed.

[awvwgk]

One surprising consequence of the current choice of list_head (0) and list_end (-1) is that appending with insert is impossible:

call list%insert(list_end + 1, "last item")

Because it will evaluate to list_head and prepend instead.

[arjenmarkus]

Yes, there is that ... I hope to solve this with special types rather than special values.

[awvwgk]

Why is -1 chosen here? The intrinsics index/scan/verify return 0 on “failure” rather than -1.

[arjenmarkus]

Yes, 0 is more consistent. Just corrected that

[awvwgk]

Suggested change

	Assignment
	Operator

[arjenmarkus]

The standard makes a difference between assignment and operator ... I can live with either, though.

[arjenmarkus]

Thanks for the comments - I will give a more detailed reply later. Op zo 3 jan. 2021 om 17:58 schreef Sebastian Ehlert < notifications@github.com>:

…

***@***.**** commented on this pull request. I looked over the API documentations and left a few comments on the API design. Mainly the special positions list_head and list_end are inconsistent in my opinion. ------------------------------ In doc/specs/stdlib_stringlist.md <#269 (comment)>: > +call list%insert( list_head, "The first string" ) +call list%insert( 20, "The last string" ) + +write(*,*) 'The last: ', list%get(list_end) +write(*,*) 'Beyond that: ', list%get(30) +``` +The special position `list_head` represents the position *before* the first element. Likewise, +the special position `list_end` represents the position of the *last* element. You can +use these positions to insert a string before the first string that is already in the +list or to insert after the last string that has been inserted. + +If you specify a position beyond the last, the `list%get()` method simply returns an empty +string. + +You can also specify *negative* positions, but they are interpreted as going back from the +last inserted string. If you need the last but one string, you can do so innthis way: ⬇️ Suggested change -last inserted string. If you need the last but one string, you can do so innthis way: +last inserted string. If you need the last but one string, you can do so in this way: ------------------------------ In doc/specs/stdlib_stringlist.md <#269 (comment)>: > +The special position `list_head` represents the position *before* the first element. Likewise, +the special position `list_end` represents the position of the *last* element. You can This feels inconsistent to me. list_head represents an element outside of the list, while list_end represents an element in the list. I would prefer either having both special indices refer to elements inside or outside the list, but not mixed. ------------------------------ In doc/specs/stdlib_stringlist.md <#269 (comment)>: > +write(*,*) 'The last: ', list%get(list_end) +write(*,*) 'Beyond that: ', list%get(30) +``` +The special position `list_head` represents the position *before* the first element. Likewise, +the special position `list_end` represents the position of the *last* element. You can +use these positions to insert a string before the first string that is already in the +list or to insert after the last string that has been inserted. + +If you specify a position beyond the last, the `list%get()` method simply returns an empty +string. + +You can also specify *negative* positions, but they are interpreted as going back from the +last inserted string. If you need the last but one string, you can do so innthis way: + +```fortran +write(*,*) 'The last but onw: ', list%get(list_end-1) ⬇️ Suggested change -write(*,*) 'The last but onw: ', list%get(list_end-1) +write(*,*) 'The last but one: ', list%get(list_end-1) ------------------------------ In doc/specs/stdlib_stringlist.md <#269 (comment)>: > + +#### Class + +Function + +#### Arguments + +`list`: the stringlist variable to retrieve a string from + +`string`: the string to be found in the list + +`back` (optional): logical argument indicating the first occurrence should be returned (`false`) or the last (`true`) + +#### Result value + +The result is either the index of the string in the list or -1 if the string was not found Why is -1 chosen here? The intrinsics index/scan/verify return 0 on “failure” rather than -1. ------------------------------ In doc/specs/stdlib_stringlist.md <#269 (comment)>: > + +#### Class + +Function + +#### Arguments + +`list`: the stringlist variable to retrieve a string from + +`substring`: the substring in question + +`back` (optional): logical argument indicating the first occurrence should be returned (`false`) or the last (`true`) + +#### Result value + +The result is either the index of the string in the list or -1 if the string was not found Same question on the return code as with index. ------------------------------ In doc/specs/stdlib_stringlist.md <#269 (comment)>: > + +Experimental + +#### Description + +Concatenate a list with a string, a list of strings or a plain array + +#### Syntax + +`concatenatedlist = list // string` + +`concatenatedlist = string // list` + +#### Class + +Assignment ⬇️ Suggested change -Assignment +Operator — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#269 (review)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAN6YRYDPU7YK2MEIS2OPQ3SYCO4FANCNFSM4VBNHQ2Q> .

[arjenmarkus]

I have been thinking about the use of integers to indicate the head and the end of the list. Clearly this is a bad design choice, as there is no way to make it unambiguous. My current idea is to use instead a special derived type, something along the lines:

type list_special_index_type
logical :: head = .true.
integer :: offset = 0
end type
with appropriate operations + and - for defining an offset. Then there can be no confusion about the meaning.

(Of course, a bit more work, but that is limited :)).

[epagone]

I have been thinking about the use of integers to indicate the head and the end of the list. Clearly this is a bad design choice, as there is no way to make it unambiguous. My current idea is to use instead a special derived type, something along the lines:

type list_special_index_type
logical :: head = .true.
integer :: offset = 0
end type
with appropriate operations + and - for defining an offset. Then there can be no confusion about the meaning.

(Of course, a bit more work, but that is limited :)).

I really like this solution: it seems to address at once most of the shortcomings that I have identified. I had a very limited amount of time to play with it but the current implementation seems to work already very well.

The aspects that I did not like much.

• As already commented in the related issue, I believe that the sorting method should not be part of this PR but it should be included in the stdlib sorting module and stdlib_stringlist should depend on it.
• I would prefer to encapsulate into the derived type the control variables (e.g. list_head, list_end in the current implementation, head and offset in the future one) to not "pollute" the user namespace. It is a bit more verbose but I think it's a better choice.
• I do not understand fully the need to hard code the initial_size = 20 parameter to a somewhat arbitrary value that also "pollutes" the user namespace (see previous point).
• I personally believe that the only way to let the user add an element to an already "initialised" list should be with the append method. Any other attempt to add new elements "out of bounds" should return an error. Quite strict, I know, but I think it's safer.

Thanks @arjenmarkus for doing this!

[ivan-pi]

@arjenmarkus I suspect you initiated this project in an old branch. The stdlib_ascii module is the old version you already modified (and is now in the master branch). The solution would be to rebase against the master branch.

[arjenmarkus]

Is it? I recently did a rebase. Okay, I will have to check. Op zo 10 jan. 2021 om 18:29 schreef Ivan Pribec <notifications@github.com>:

…

@arjenmarkus <https://github.com/arjenmarkus> I suspect you initiated this project in an old branch. The stdlib_ascii module is the old version you already modified (and is now in the master branch). The solution would be to rebase against the master branch. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#269 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAN6YR4BPWLFYPSB23YLEP3SZHPYHANCNFSM4VBNHQ2Q> .

[milancurcic]

Closing in favor of #311.