Upgrade dcat ap3 #80
Upgrade dcat ap3 #80
Conversation
Reviewer's Guide by SourceryThis pull request upgrades the CKAN extension to comply with DCAT AP 3 standard, which involves significant changes to the schema, preset files, and unit tests. The main focus is on updating the data model and parsing logic to match the new DCAT AP 3 requirements. Sequence DiagramsequenceDiagram
participant Client
participant FAIRDataPointDCATAPProfile
participant EuropeanDCATAP3Profile
Client->>FAIRDataPointDCATAPProfile: parse_dataset(dataset_dict, dataset_ref)
FAIRDataPointDCATAPProfile->>EuropeanDCATAP3Profile: super().parse_dataset(dataset_dict, dataset_ref)
FAIRDataPointDCATAPProfile->>FAIRDataPointDCATAPProfile: _parse_creator(dataset_dict, dataset_ref)
FAIRDataPointDCATAPProfile->>FAIRDataPointDCATAPProfile: validate_tags(dataset_dict['tags'])
FAIRDataPointDCATAPProfile-->>Client: Updated dataset_dict
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey @hcvdwerf - I've reviewed your changes - here's some feedback:
Overall Comments:
- Consider revisiting the commented-out tests in test_profiles.py to ensure adequate test coverage is maintained after the DCAT AP 3 upgrade.
Here's what I looked at during the review
- 🟢 General issues: all looks good
- 🟢 Security: all looks good
- 🟡 Testing: 4 issues found
- 🟢 Complexity: all looks good
- 🟡 Documentation: 1 issue found
Help me be more useful! Please click 👍 or 👎 on each comment to tell me if it was helpful.
| ], | ||
| 'title': '[PUBLIC] Low-Grade Gliomas (UCSF, Science 2014)', | ||
| 'notes': 'Whole exome sequencing of 23 grade II glioma tumor/normal pairs.', | ||
| 'url': 'https://cbioportal.health-ri.nl/study/summary?id=lgg_ucsf_2014', |
There was a problem hiding this comment.
issue (testing): Date format changed from datetime object to string
The test has been updated to expect a string representation of the date instead of a datetime object. Ensure that this change is consistent with the new requirements and that the application can handle this format correctly.
| 'modified': datetime(2019, 10, 30, 23, 0), | ||
| 'issued': '2019-10-30 23:00:00', | ||
| 'modified': '2019-10-30 23:00:00', | ||
| 'identifier': 'lgg_ucsf_2014', |
There was a problem hiding this comment.
issue (testing): Publisher structure changed from string to list of objects
The test has been updated to reflect a change in the structure of the 'publisher' field. It's now a list of objects instead of a single string. This change should be verified against the new schema requirements.
| "issued": '2023-10-06T10:12:55.614000+00:00', | ||
| "language": ["http://id.loc.gov/vocabulary/iso639-1/en"], | ||
| "license_id": "", | ||
| "modified": datetime(2023, 10, 6, 10, 12, 55, 614000, tzinfo=tzutc()), | ||
| "publisher_name": "Automatic", | ||
| "modified": '2023-10-06T10:12:55.614000+00:00', |
There was a problem hiding this comment.
issue (testing): Date format changed to ISO 8601 string
The test now expects dates in ISO 8601 string format instead of datetime objects. Verify that this change is consistent across the application and that all date handling code can work with this format.
| pip install -e 'git+https://github.com/ckan/ckanext-dcat.git@v2.0.0#egg=ckanext-dcat' | ||
| pip install -r https://raw.githubusercontent.com/ckan/ckanext-dcat/v2.0.0/requirements.txt |
There was a problem hiding this comment.
question (documentation): Consider if other documentation needs updating due to ckanext-dcat version change
The update from v1.5.1 to v2.0.0 of ckanext-dcat might introduce significant changes. Have you reviewed if any other parts of the documentation or codebase need to be updated to reflect this change?
🚀 Pull Request Checklist
Title:
[x]feat: Upgrade to DCAT AP 3Description:
[x]This pull request upgrades the schema, and preset files to comply with DCAT AP 3. Fix of unit tests. At the moment there is a bug in the DCAT extension that not correctly handles with multiple contact and publishersContext:
[x]These changes are necessary to bring the system in line with the latest DCAT AP 3 standard, improving metadata handling and making the system compatible with new regulatory requirements. This resolves issue #122.Changes:
[x]Fix UT according to new schema[x]Remove obsolete code[x]Update schema and preset files to match DCAT AP 3[x]Use new DCAT extensionTesting:
[x]Changes have been tested locally on the development environment. Unit tests have been run and updated to reflect the schema changes. Functional tests on Solr have passed without issues.Screenshots (if applicable):
[ ]N/AAdditional Information:
[ ]NoneChecklist:
[x]I have checked that my code adheres to the project's style guidelines and that my code is well-commented.[x]I have performed self-review of my own code and corrected any misspellings.[x]I have made corresponding changes to the documentation (if applicable).[x]My changes generate no new warnings or errors.[x]I have added tests that prove my fix is effective or that my feature works.[x]New and existing unit tests pass locally with my changes.Summary by Sourcery
Upgrade the system to comply with DCAT AP 3 by updating schema and preset files, fixing unit tests, and removing obsolete code. Update CI workflows and documentation to reflect these changes, ensuring compatibility with the latest standards and regulatory requirements.
New Features:
Bug Fixes:
Enhancements:
CI:
Documentation:
Tests:
Chores: