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

XML2D - multivalue keys #52

Closed
scleakey opened this issue Apr 3, 2023 · 2 comments
Closed

XML2D - multivalue keys #52

scleakey opened this issue Apr 3, 2023 · 2 comments
Assignees
Labels
bug Something isn't working

Comments

@scleakey
Copy link
Collaborator

scleakey commented Apr 3, 2023

I think the except part might need some sort of warning in

if elem_key in self._multi_value_keys:
if not list_idx_key == elem_key:
list_idx_key = elem_key
list_idx = 0
try:
self._recursive_remove_data_xml(new_dict[elem_key][list_idx], elem)
list_idx += 1
except (IndexError, KeyError):
parent.remove(elem)

I was trying to set the link1d attribute but didn't realise it was in self._multi_value_keys so this would always fail silently, resulting in the links being completely absent in the saved xml file.

I realise this is kind of implied by:

- ``.link1d`` - A list of 1 or more 1D links, each containing a dictionary of parameters
including the link shapefile and ief file.

and
Any option where multiple values can be provided (e.g. topography files) will be present
as a list of dictionaries, whereas options where only one value can be required will be
a single dictionary.

but it could be clearer.

@scleakey scleakey added the bug Something isn't working label Apr 3, 2023
scleakey added a commit that referenced this issue Apr 5, 2023
@scleakey scleakey self-assigned this Apr 5, 2023
scleakey added a commit that referenced this issue Apr 5, 2023
@joe-pierce joe-pierce self-assigned this Apr 5, 2023
joe-pierce added a commit that referenced this issue Apr 6, 2023
@joe-pierce
Copy link
Collaborator

I've commited some changes that should capture when elements are not added as a list when they need to be, this needed to be done in the recursive update as well as the adding new keys in case an existing element is replaced with a non-list value.

I've also added a line that restores the xmltree in cases of errors. Also needed to update one of the tests to add elements as a list.

@scleakey
Copy link
Collaborator Author

I had to change my code from

def edit_file(self) -> None:
self._xml.domains[self._domain_name]["time"] = {
"start_offset": self._start_offset,
"total": self._total,
}
self._xml.domains[self._domain_name]["run_data"] = {
"time_step": self._time_step,
"scheme": self._scheme,
}
self._xml.processor = {"type": self._processor}
to
def edit_file(self) -> None:
self._xml.domains[self._domain_name]["time"] = {
"start_offset": {"value": self._start_offset},
"total": {"value": self._total},
}
self._xml.domains[self._domain_name]["run_data"] = {
"time_step": self._time_step,
"scheme": {"value": self._scheme},
}
self._xml.processor = {"type": self._processor}
to get it to work after merging from main with your new commits. Is this intended?

joe-pierce added a commit that referenced this issue Apr 12, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants