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

New attribute literal label set as Optional #766

Merged
merged 3 commits into from
Apr 3, 2023

Conversation

SarahJohnsonONS
Copy link
Contributor

Bug fix - changed NewAttributeLiteral label to Optional[str] - if label not provided, it defaults to the column title.

@github-actions
Copy link

github-actions bot commented Mar 28, 2023

ubuntu-latest-python3.11 test results

527 tests  +1   527 ✔️ +1   3m 56s ⏱️ +8s
    9 suites ±0       0 💤 ±0 
    9 files   ±0       0 ±0 

Results for commit 32e7c8e. ± Comparison against base commit 7f33ebe.

♻️ This comment has been updated with latest results.

@github-actions
Copy link

github-actions bot commented Mar 28, 2023

ubuntu-latest-python3.9 test results

527 tests  +1   527 ✔️ +1   3m 15s ⏱️ +7s
    9 suites ±0       0 💤 ±0 
    9 files   ±0       0 ±0 

Results for commit 32e7c8e. ± Comparison against base commit 7f33ebe.

♻️ This comment has been updated with latest results.

@github-actions
Copy link

github-actions bot commented Mar 28, 2023

windows-latest-python3.11 test results

  10 files    11 suites   6m 46s ⏱️
541 tests 541 ✔️ 0 💤 0
554 runs  554 ✔️ 0 💤 0

Results for commit 32e7c8e.

♻️ This comment has been updated with latest results.

Copy link
Contributor

@CharlesRendle CharlesRendle left a comment

Choose a reason for hiding this comment

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

Happy that this change satisfies the ticket.
Have verified that the column name is used for label property when building a csvw
And it looks like the documentation portion has been addressed in ticket #520
Approving, but worth thinking if this needs a unit test to demonstrate what the problem is/was and the functionality now expected under the bug conditions. E.g.

When an attribute column is defined:

"columns": {
    "My New Attribute Literal": {
         "type": "Attribute",
         "data_type": "decimal",
    }
}

The metadata.json file should contain:

 "http://www.w3.org/2000/01/rdf-schema#label": [
                {
                    "@language": "en",
                    "@value": "Attribute"
                }
            ],

@robons given it's such a small change, what do you think?

Copy link
Contributor

@CharlesRendle CharlesRendle left a comment

Choose a reason for hiding this comment

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

Spoken with Rob - please can you add a unit test to demonstrate the issue/fix for this ticket.

@sonarcloud
Copy link

sonarcloud bot commented Mar 31, 2023

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

0.0% 0.0% Coverage
0.0% 0.0% Duplication

Copy link
Contributor

@CharlesRendle CharlesRendle left a comment

Choose a reason for hiding this comment

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

All looks good to me!
Thank you for writing the test

@NickPapONS NickPapONS merged commit 6e985ce into main Apr 3, 2023
@NickPapONS NickPapONS deleted the #759-Attribute-Literal-`Label`-has-no-default branch April 3, 2023 09:21
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