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

add regex validation rule to filename attributes #145

Merged
merged 16 commits into from
May 8, 2023
Merged

Conversation

brynnz22
Copy link
Contributor

@brynnz22 brynnz22 commented Jan 20, 2023

Added the validation rule: regex search ^\S*$ to Filename and Channel Metadata Filename attributes. I was not sure about adding it to the 'Channel Metadata Filename attribute, so I can delete this if needed. Addressing issue #139

@adamjtaylor adamjtaylor linked an issue Jan 23, 2023 that may be closed by this pull request
Copy link
Contributor

@adamjtaylor adamjtaylor left a comment

Choose a reason for hiding this comment

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

Thanks @brynnz22! Looks like we managed to drag some extra quotes in that we will want to fix so that only lines 8 and 132 have changes in. If we do want to standardise quoting descriptions (and that might be a good idea we can do this through a different issue/PR.
I'm going to hold off on approving and merging for now and will try and deploy this branch into a testing instance

HTAN.model.csv Outdated Show resolved Hide resolved
HTAN.model.csv Outdated Show resolved Hide resolved
HTAN.model.csv Show resolved Hide resolved
HTAN.model.csv Outdated Show resolved Hide resolved
brynnz22 and others added 6 commits January 23, 2023 16:37
Delete leading and trailing white spaces in filename and scDNA-seq Level 1 descriptions.
Delete extra quotes and spaces.
Delete quotes and leading and trailing white spaces, including "url"
Delete leading and trailing quotes/white spaces for is lowest level
@brynnz22
Copy link
Contributor Author

@adamjtaylor I implemented all the fixes. It should be good.

adamjtaylor
adamjtaylor previously approved these changes Jan 26, 2023
Copy link
Contributor

@adamjtaylor adamjtaylor left a comment

Choose a reason for hiding this comment

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

Looks good!

@adamjtaylor
Copy link
Contributor

Thanks @brynnz22. This looks good. I will redeploy to testing before we merge here.

@adamjtaylor
Copy link
Contributor

I'm struggling to get this to deploy currently as the develop schematic does not install properly.
See action: https://github.com/ncihtan/HTAN-data-curator/actions/runs/3989248484
Will take another look later, otherwise to escalate to FAIR data.
Maybe we can try and test the feature just in the schematic CLI first

@afwillia
Copy link

@adamjtaylor there was an update to schematic yesterday that made it incompatible with python 3.8, which our deployment workflow uses. I'm working to update the deployment to use python 3.10 in this branch and I'll let you know when testing is complete.

@aclayton555
Copy link
Contributor

Hold on this until next data model release

@brynnz22
Copy link
Contributor Author

brynnz22 commented Feb 3, 2023

This successfully converts to a json.ld using the Schematic CLI (schematicpy 23.1.1).

@brynnz22
Copy link
Contributor Author

brynnz22 commented Feb 3, 2023

I tried validating a manifest using the CLI and I'm not sure what is going on. It doesn't seem to recognize the entire Filename column as it returns a KeyError: Filename

@brynnz22
Copy link
Contributor Author

brynnz22 commented Feb 3, 2023

I can test this out more when I am back on the 21st.

@adamjtaylor
Copy link
Contributor

Removing from the project board as is tracked in the linked issue #139

@adamjtaylor adamjtaylor self-requested a review March 9, 2023 11:56
@adamjtaylor adamjtaylor dismissed their stale review March 9, 2023 11:57

PR cannot be merged due to key error. Re-review once #139 unbocked

@adamjtaylor adamjtaylor self-assigned this Mar 9, 2023
@adamjtaylor adamjtaylor changed the title add regex validation rule to filename attributes add regex validation rule to filename attributes (DO NOT INCLUDE IN RELEASE. FOR STAGING ONLY) May 5, 2023
@elv-sb elv-sb merged commit 9d877e7 into main May 8, 2023
@elv-sb elv-sb deleted the no-space-filenames branch May 8, 2023 18:42
@adamjtaylor adamjtaylor changed the title add regex validation rule to filename attributes (DO NOT INCLUDE IN RELEASE. FOR STAGING ONLY) add regex validation rule to filename attributes May 9, 2023
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.

Enforce no spaces in file names
5 participants