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

Hashap key generic interfaces #827

Merged
merged 16 commits into from
Jun 19, 2024

Conversation

chuckyvt
Copy link
Contributor

This is update to the hashmap capabilities to include generic key interfaces for most of the type-bound procedures that use key type. I believe this will be a nice step forward for the user interface, reducing number of lines of code to use the hashamaps, and makes it closer to the style of a dictionary in Python.

For example, currently to map an entry:

call set( key, [1] )
call map % map_entry( key, other, conflict )

This simplifies to
call map % map_entry( [1], other, conflict )

This is the initial submit with src and testing updated for review. Will work on specs and adding examples next.

Included tests in test_maps.fypp for generic key interfaces.
Removed un-needed import statements that were causing compile issues with some compilers.
@chuckyvt chuckyvt marked this pull request as draft May 24, 2024 13:27
Updated specs and examples to include generic key interface examples.
@chuckyvt chuckyvt marked this pull request as ready for review May 31, 2024 04:03
@chuckyvt
Copy link
Contributor Author

I believe this is ready for review. There was a single CI failure that I don't understand, but doesn't appear to be related to the code updates?

@chuckyvt
Copy link
Contributor Author

Also worth mentioning that some of the hashmap examples were somewhat outdated, assuming the hashmap capabilities have matured since the examples were written. In one example there was a routine written, key_to_char, to convert a key back to a character string. Since the get function does this exact task, the example was updated to use the get function instead of using a custom function.

@jvdp1
Copy link
Member

jvdp1 commented Jun 1, 2024

Is this PR related to #664 ? (I don't think so, but just to be sure).

@chuckyvt
Copy link
Contributor Author

chuckyvt commented Jun 2, 2024

They are unrelated. This PR only affects the 'key' field. No changes to 'other'.

Minor reordering of code to make more readable.
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 @chuckyvt . Overall LGTM. IMO it could be merged, pending minor changes.

doc/specs/stdlib_hashmaps.md Outdated Show resolved Hide resolved
doc/specs/stdlib_hashmaps.md Outdated Show resolved Hide resolved
doc/specs/stdlib_hashmaps.md Outdated Show resolved Hide resolved
example/hashmaps/example_hashmaps_get_other_data.f90 Outdated Show resolved Hide resolved
example/hashmaps/example_hashmaps_map_entry.f90 Outdated Show resolved Hide resolved
example/hashmaps/example_hashmaps_remove.f90 Outdated Show resolved Hide resolved
example/hashmaps/example_hashmaps_set_other_data.f90 Outdated Show resolved Hide resolved
src/stdlib_hashmaps.f90 Outdated Show resolved Hide resolved
src/stdlib_hashmaps.f90 Outdated Show resolved Hide resolved
test/hashmaps/test_maps.fypp Outdated Show resolved Hide resolved
chuckyvt and others added 10 commits June 9, 2024 15:34
Co-authored-by: Jeremie Vandenplas <jeremie.vandenplas@gmail.com>
Co-authored-by: Jeremie Vandenplas <jeremie.vandenplas@gmail.com>
Co-authored-by: Jeremie Vandenplas <jeremie.vandenplas@gmail.com>
Co-authored-by: Jeremie Vandenplas <jeremie.vandenplas@gmail.com>
Co-authored-by: Jeremie Vandenplas <jeremie.vandenplas@gmail.com>
Co-authored-by: Jeremie Vandenplas <jeremie.vandenplas@gmail.com>
Co-authored-by: Jeremie Vandenplas <jeremie.vandenplas@gmail.com>
Co-authored-by: Jeremie Vandenplas <jeremie.vandenplas@gmail.com>
Co-authored-by: Jeremie Vandenplas <jeremie.vandenplas@gmail.com>
Renaming of char_type variable as _type is typical reserved for derived types.
@chuckyvt
Copy link
Contributor Author

Thanks for the detailed review. I believe all requested changes have been included.

@jvdp1
Copy link
Member

jvdp1 commented Jun 16, 2024

@chuckyvt : Could you rebase this PR, please (to check if it solves the CI issue)?

@jvdp1
Copy link
Member

jvdp1 commented Jun 17, 2024

Thank you @chuckyvt . I will wait a couple of days before merging, in case of additional comments.

@jvdp1 jvdp1 merged commit d996e43 into fortran-lang:master Jun 19, 2024
17 checks passed
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.

None yet

2 participants