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

Feature/fix geog dimension metadata #66

Merged
merged 4 commits into from
Jan 10, 2023

Conversation

andre-urbani
Copy link
Contributor

@andre-urbani andre-urbani commented Jan 5, 2023

What

Fix for updated geography dimension not being added to metadata on filtered dataset

This fix is to address the issue with the original PR for this ticket (#62), which when tested in sandbox was still not pulling through the updated geography dimension in the csvw and txt metadata files

Trello - https://trello.com/c/MGF4L4rT/5950-get-meta-data-for-area-type-selection-to-show-on-custom-data-set-output-files

How to review

Confirm changes make sense and match requested changes in Trello ticket

Who can review

Anyone

break
}
}

if !areaTypeFound {
areaTypeDimension := []dataset.VersionDimension{
Copy link
Contributor

Choose a reason for hiding this comment

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

Does this need to be a slice?

for _, d := range m.Version.Dimensions {
if *d.IsAreaType {
d.Label = areaTypeLabel
d.Description = areaTypeDescription
d.ID = areaTypeId
Copy link
Contributor

Choose a reason for hiding this comment

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

Are you 100% sure this is working as expected? I'd half expect this not to update the dimensions of the original struct you're trying to, I might be wrong though.

@andre-urbani andre-urbani merged commit 469288b into develop Jan 10, 2023
@andre-urbani andre-urbani deleted the feature/fix-geog-dimension-metadata branch January 10, 2023 11:00
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