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

added a new function: get_value_in_nested_dict #31

Open
wants to merge 1 commit into
base: release
Choose a base branch
from

Conversation

denizural
Copy link
Contributor

I added this functionality to get the list of values from a dictionary for a given key. There is already a function called find_key but it does not do what I want it to do and even more, I think there a some odd behaviour in that function. Look at this test:

from esm_parser import find_key
from esm_parser import get_value_in_nested_dict
import yaml

# esm_tools/configs/machines/generic.yaml
with open('generic.yaml') as f:    
    data = yaml.load(f, Loader=yaml.FullLoader)
      
    mykey = "fc"

    search_list = find_key(data, [mykey], exc_strings = "", level = "", paths2finds = [], sep = ".")
    print("Result of find_key:")
    print(search_list)
        
    search_list_2 = get_value_in_nested_dict(data, mykey)
    print("Result of get_value_in_nested_dict:")
    print(list(search_list_2))

then the result is

Result of find_key:
['choose_useMPI.intelmpi.fc', 'choose_useMPI.intelmpi.mpifc', 'choose_useMPI.openmpi.fc', 'choose_useMPI.openmpi.mpifc']

Result of get_value_in_nested_dict:
['mpiifort', 'mpif90']

The previous function (find_key) even matches mpifc. Is this the expected behaviour? I think the in operator is causing the trouble here. I think == operator should be use to compare the strings.

In any case I think this new function will be useful for the future cases.

@denizural denizural requested review from pgierz and mandresm March 8, 2021 06:39
@pgierz
Copy link
Member

pgierz commented Mar 8, 2021

Hi Deniz,

since you are writing manual tests, I would suggest instead to start slowly incorporating those into a unit test suite, which can run automatically whenever someone changes the parser.

@mandresm
Copy link
Contributor

mandresm commented Mar 8, 2021

@denizural , I put together this function some time ago to search for the key word _changes, excluding add_. This is used mainly in the yaml_to_dict method, but I am also using it right now for the environment rework. For all of that you need an in instead of the ==.

@denizural
Copy link
Contributor Author

@pgierz, I totally agree with you. I think we need more test driven development in the project for less headaches 😄. Python already provides unit testing frameworks. I will have a look at these.

@mandresm, thank you very much for the explanation. I another branch that I am currently working, I needed this functionality and decided to search the parser if we already have something to avoid duplicate code.

@mandresm
Copy link
Contributor

mandresm commented Mar 8, 2021

Sure :) I also need this functionality for the environment rework, in fact I coded something simpler for nested keys, but your solution is better and more general so I will switch to yours as soon as it is available in develop and release.

About find_key I'd like to clarify a bit more why I needed at that time: The main point of this function is that it gives you the whole path to keys sharing a string. This is useful for example to search for all namelist_changes and add_namelist_changes in a yaml file and understand which ones are included in a choose_ block and check if they follow the rules of declaring namelist_changes. For that you don't need the value itself, but the path to the key.

@mandresm
Copy link
Contributor

mandresm commented Mar 8, 2021

@denizural , so this is the little function I put together to use the paths provided by find_key to get the value of a nested key. By giving values to level you can "climb up" levels on the nested dictionary, starting from the key.

def get_nested_key(dictionary, keys, level = 0):
    if len(keys)>level:
        key = keys.pop(0)
        var = get_nested_key(dictionary[key], keys, level)
    else:
        var = dictionary
    return var

Let me know if you think it worths to include this functionality here so that this function is also compatible with find_key searches.

@mandresm
Copy link
Contributor

@denizural , so this is the little function I put together to use the paths provided by find_key to get the value of a nested key. By giving values to level you can "climb up" levels on the nested dictionary, starting from the key.

def get_nested_key(dictionary, keys, level = 0):
    if len(keys)>level:
        key = keys.pop(0)
        var = get_nested_key(dictionary[key], keys, level)
    else:
        var = dictionary
    return var

Let me know if you think it worths to include this functionality here so that this function is also compatible with find_key searches.

I just found out that there was an already existing method for this purpose on esm_parser so the lines I asked you to include are not needed anymore. I can do without them using the existing method find_value_for_nested_key.

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.

3 participants