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.

Sign up for GitHub

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

Jump to bottom

Add validation for the new CNA Long Format (with custom namespace columns) #9882

Merged
merged 3 commits into from
Nov 14, 2022
Merged

Add validation for the new CNA Long Format (with custom namespace columns) #9882

merged 3 commits into from
Nov 14, 2022

Conversation

oplantalech
Copy link
Contributor

@oplantalech oplantalech commented Nov 9, 2022 •
edited
Loading

Validation part for #9847

@pvannierop pvannierop changed the title Add validation for the new CNA Long Format (with cutom namespace columns) Add validation for the new CNA Long Format (with custom namespace columns) Nov 9, 2022
@inodb inodb requested a review from dippindots November 9, 2022 15:45
dippindots
dippindots reviewed Nov 11, 2022
View reviewed changes
Copy link
Member

@dippindots dippindots left a comment

Choose a reason for hiding this comment

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

@oplantalech Thanks for implementing this, overall it looks good to me, just have some minor comments, please comment below.
It's also good to have documentation in File-Format file to give everyone an idea of how to use this new file format.

core/src/main/scripts/importer/validateData.py Outdated Show resolved Hide resolved
core/src/main/scripts/importer/validateData.py Outdated Show resolved Hide resolved
core/src/main/scripts/importer/validateData.py Show resolved Hide resolved
core/src/test/scripts/unit_tests_validate_data.py Outdated Show resolved Hide resolved
@oplantalech
Copy link
Contributor Author

@dippindots Regarding when to fix the File-Formats page, I've discussed it with @pvannierop and we thing it is better to have it done after the functionality has been merged and it is working.

dippindots
dippindots reviewed Nov 11, 2022
View reviewed changes
Copy link
Member

@dippindots dippindots left a comment

Choose a reason for hiding this comment

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

Thanks for the quick update, I have two more comments, after we fixed these two, I think we can merge it.

@sonarcloud
Copy link

sonarcloud bot commented Nov 14, 2022

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

No Coverage information No Coverage information
No Duplication information No Duplication information

dippindots
dippindots approved these changes Nov 14, 2022
View reviewed changes
Copy link
Member

@dippindots dippindots left a comment

Choose a reason for hiding this comment

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

@oplantalech It looks good to me, thanks for fixing all the comments.

@dippindots dippindots added validator cl-prototype Prototype section in changelog. New features not ready for production use labels Nov 14, 2022
@dippindots dippindots merged commit 1ee6c9d into cBioPortal:master Nov 14, 2022
@oplantalech oplantalech deleted the validate-cna-long-format branch November 15, 2022 08:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dippindots dippindots dippindots approved these changes

@pvannierop pvannierop Awaiting requested review from pvannierop

Assignees
No one assigned
Labels
cl-prototype Prototype section in changelog. New features not ready for production use validator
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@oplantalech @dippindots