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

HDX-9984 Allow key specification to be nested #34

Merged
merged 14 commits into from
Jul 3, 2024

Conversation

IanHopkinson
Copy link
Collaborator

@IanHopkinson IanHopkinson commented Jul 1, 2024

Purpose

Version for this PR: 2024.7.1

This PR provides functionality to access nested keys in the list command, typically the nested keys of the organization dictionary, or the keys of lists such as tags and groups.

#5

Major file changes

The major change is the introduction of a new function, query_dict which is added to utilities.py and tested in test_utilities.py. It is invoked in the "list" command in cli.py:

@hdx_toolkit.command(name="list")

Minor file changes

As a result of changing the global git config handling line endings, all(?) many lines of code appeared to have changed but I believe this is just a line ending change.

Versioning

hdx-cli-toolkit uses the CalVer versioning scheme with format YYYY.MM.Micro i.e. 2022.12.1 which is updated manually in pyproject.toml. The "Micro" component is simply an integer increased by 1 at each version, starting from 0.

  • Version updated in pyproject.toml and PR description
  • Update README.md and DEMO.md with any new CLI commands

@IanHopkinson IanHopkinson changed the title HDX-9984 HDX-9984 Allow key specification to be nested Jul 1, 2024
@IanHopkinson IanHopkinson requested a review from turnerm July 1, 2024 15:24
@IanHopkinson
Copy link
Collaborator Author

Pipeline failures are mainly down to the stage API key changing a couple of times!

@IanHopkinson IanHopkinson changed the base branch from main to git-line-endings July 2, 2024 08:54
Copy link
Member

@turnerm turnerm left a comment

Choose a reason for hiding this comment

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

I'm glad I got to do this review, it was my first time using the hdx toolkit, and it's really slick, I'll definitely be keeping it in my back pocket!

Overall I think this change is a tricky one because it opens up a lot of different possibilities for queries. Below are some ones that I tried:

First, there appears to be a duplication bug, e.g. the following returns two rows:

hdx-toolkit list \
  --dataset_filter=gibraltar-healthsites --hdx_site=stage \
  --key=archived,batch

I suspect it comes from using both output and output_row in the query_dict function.

This returns a dictionary that doesn't fit into the table:

hdx-toolkit list \
  --dataset_filter=gibraltar-healthsites --hdx_site=stage \
  --key=resources \
  --with_extras

This returns a too many values error that should perhaps be caught or otherwise handled:

hdx-toolkit list \
  --dataset_filter=gibraltar-healthsites \
  --hdx_site=stage \
  --key=resources.fs_check_info.state \
  --with_extras

This returns 5 rows of absent key:

hdx-toolkit list \
  --dataset_filter=gibraltar-healthsites \
  --hdx_site=stage \
  --key=resources.nonsense \
  --with_extras

pyproject.toml Show resolved Hide resolved
src/hdx_cli_toolkit/utilities.py Outdated Show resolved Hide resolved
src/hdx_cli_toolkit/utilities.py Show resolved Hide resolved
src/hdx_cli_toolkit/utilities.py Outdated Show resolved Hide resolved
src/hdx_cli_toolkit/utilities.py Outdated Show resolved Hide resolved
tests/test_utilities.py Show resolved Hide resolved
DEMO.md Show resolved Hide resolved
src/hdx_cli_toolkit/cli.py Outdated Show resolved Hide resolved
src/hdx_cli_toolkit/utilities.py Outdated Show resolved Hide resolved
src/hdx_cli_toolkit/utilities.py Outdated Show resolved Hide resolved
@IanHopkinson
Copy link
Collaborator Author

@turnerm here's my response to your test cases:

  1. archived,batch – produced two lines a bug – FIXED
  2. resources – produced a dictionary that didn’t fit in the table – it will output correctly to file – FIXED by providing an information message:
    Field 'resources' is list or dict type, use --output_path to see full output
  3. resources.fs_check_info.state – fails because it is 3 keys deep and first two are lists – FIXED by providing message:
    resources.fs_check_info.state is nested to depth 3, maximum depth is 2
  4. resources.nonsense – returns five rows with “absent key” – I think this behaviour is correct, we may have situations where some items in the list have the key but others don't - WONTFIX

@turnerm turnerm self-requested a review July 3, 2024 14:34
@IanHopkinson IanHopkinson merged commit b28a800 into git-line-endings Jul 3, 2024
@IanHopkinson IanHopkinson deleted the HDX9948_nested_keys branch July 3, 2024 15:24
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.

2 participants