Enforce no spaces in file names #139

adamjtaylor · 2022-12-22T18:19:29Z

This has caused a number of downstream issues in the past and we should enforce with a regex validation rule

aclayton555 · 2023-01-09T19:32:41Z

Please work with @brynnz22 on this.

Do we enforce no spaces? no white space characters at all? no special characters?

adamjtaylor · 2023-01-20T15:45:29Z

the regex string ^\S*$ should be appropriate to capture start of string, any number of non-whitespace characters followed by the end of the string

adamjtaylor · 2023-01-20T15:48:09Z

From the docs this can be implemented as adding regex match ^\S*$ to the Validation Rules column for the attribute Filename

adamjtaylor · 2023-01-20T15:49:25Z

@brynnz22 do you think you try and open a PR to modify the data model to add the validation rule above. Happy to do so in a working session if preferable. We can try and deploy this branch to a testing instance and see the rule works appropriately.

aclayton555 · 2023-01-20T17:44:20Z

Note to Brynn: Make a new branch titled based on this, then open a PR against main. Once PR done, work with Adam to run action: Should be able to run gh action on that branch to deploy to testing1

adamjtaylor · 2023-01-23T18:50:03Z

I've deployed this to the testing1 instance

To do this I:

Made a branch for HTAN-data-curator called develop-nospace-validation
edited the schematic_config.yml file to point towards the no-space-filenames branch of the data-models repo
This then automatically kicks off the shiny_deploy action. As it is started from a branch prefixed with develop, it deploys to the testing1 instance

Of course this will all change for the upcoming multi-tennant DCA

adamjtaylor · 2023-01-23T19:03:10Z

I'm getting a DCA disconnect when I try and generate a template in the testing instance. Will look at logs and see if I can see what the issue is.

adamjtaylor · 2023-01-23T19:49:39Z

Nothing in the logs to suggest the cause 😪

brynnz22 · 2023-01-23T20:31:32Z

@adamjtaylor Is it the weird quotes and things that were inserted in the csv that could be causing this?

adamjtaylor · 2023-01-23T20:37:38Z

@brynnz22 Yeah I think it might be the quoted validation rule "url " highlighted in my review of the PR. Lets remove them (editing the csv directly in the Github web UI might be the easiest way to avoid any local formatting issues) and then we can redeploy to testing1 Thank you!

adamjtaylor · 2023-02-22T15:09:42Z

Hey @brynnz22 I've been trying to test this validation rule locally with a small test template based from OtherAssay

Component,Filename,File Format,HTAN Parent Biospecimen ID,HTAN Data File ID,Assay Type
OtherAssay,underscore_separated.csv,csv,HTA1_1_1,HTA1_2_2,Test
OtherAssay,space separated.csv,csv,HTA1_1_1,HTA1_2_2,Test

This should fail validation due to the space.... but schematic seems to trip up with error KeyError: 'Filename'

(data-models) ataylor@w152 test-nospace-validation % schematic model -c schematic_config.yml validate -mp other_assay_example.csv -dt "OtherAssay"
Starting schematic...
The (model > input > location) argument with value 'HTAN.model.jsonld' is being read from the config file.
The (model > input > file_type) argument with value 'local' is being read from the config file.
JSON schema successfully generated from schema.org schema!
JSON schema file log stored as HTAN.OtherAssay.schema.json
FileDataContext loading zep config
GxConfig.parse_yaml() failed with errors - [{'loc': ('xdatasources',), 'msg': 'field required', 'type': 'value_error.missing'}]
GxConfig.parse_yaml() returning empty `xdatasources`
Loading 'datasources' ->
{}
Loaded 'datasources' ->
{}
EphemeralDataContext has not implemented `_load_zep_config()` returning empty `GxConfig`
Loading 'datasources' ->
{}
Loaded 'datasources' ->
{}
warning: /Users/ataylor/miniforge3/envs/data-models/lib/python3.10/site-packages/great_expectations/data_context/data_context/abstract_data_context.py:2590: DeprecationWarning: create_expectation_suite is deprecated as of v0.15.48 and will be removed in v0.18. Please use add_expectation_suite or add_or_update_expectation_suite instead.
warning:   warnings.warn(
warning: /Users/ataylor/miniforge3/envs/data-models/lib/python3.10/site-packages/great_expectations/data_context/data_context/abstract_data_context.py:373: DeprecationWarning: save_expectation_suite is deprecated as of v0.15.48 and will be removed in v0.18. Please use update_expectation_suite or add_or_update_expectation_suite instead.
warning:   warnings.warn(
        0 expectation(s) included in expectation_suite.
Calculating Metrics: 0it [00:00, ?it/s]
Traceback (most recent call last):
  File "/Users/ataylor/miniforge3/envs/data-models/bin/schematic", line 8, in <module>
    sys.exit(main())
  File "/Users/ataylor/miniforge3/envs/data-models/lib/python3.10/site-packages/click/core.py", line 1130, in __call__
    return self.main(*args, **kwargs)
  File "/Users/ataylor/miniforge3/envs/data-models/lib/python3.10/site-packages/click/core.py", line 1055, in main
    rv = self.invoke(ctx)
  File "/Users/ataylor/miniforge3/envs/data-models/lib/python3.10/site-packages/click/core.py", line 1657, in invoke
    return _process_result(sub_ctx.command.invoke(sub_ctx))
  File "/Users/ataylor/miniforge3/envs/data-models/lib/python3.10/site-packages/click/core.py", line 1657, in invoke
    return _process_result(sub_ctx.command.invoke(sub_ctx))
  File "/Users/ataylor/miniforge3/envs/data-models/lib/python3.10/site-packages/click/core.py", line 1404, in invoke
    return ctx.invoke(self.callback, **ctx.params)
  File "/Users/ataylor/miniforge3/envs/data-models/lib/python3.10/site-packages/click/core.py", line 760, in invoke
    return __callback(*args, **kwargs)
  File "/Users/ataylor/miniforge3/envs/data-models/lib/python3.10/site-packages/click/decorators.py", line 38, in new_func
    return f(get_current_context().obj, *args, **kwargs)
  File "/Users/ataylor/miniforge3/envs/data-models/lib/python3.10/site-packages/schematic/models/commands.py", line 209, in validate_manifest
    errors, warnings = metadata_model.validateModelManifest(
  File "/Users/ataylor/miniforge3/envs/data-models/lib/python3.10/site-packages/schematic/models/metadata.py", line 254, in validateModelManifest
    errors, warnings, manifest = validate_all(self, errors, warnings, manifest, manifestPath, self.sg, jsonSchema, restrict_rules, project_scope)
  File "/Users/ataylor/miniforge3/envs/data-models/lib/python3.10/site-packages/schematic/models/validate_manifest.py", line 253, in validate_all
    manifest, vmr_errors, vmr_warnings = vm.validate_manifest_rules(manifest, sg, restrict_rules, project_scope)
  File "/Users/ataylor/miniforge3/envs/data-models/lib/python3.10/site-packages/schematic/models/validate_manifest.py", line 208, in validate_manifest_rules
    vr_errors, vr_warnings = validation_method(
  File "/Users/ataylor/miniforge3/envs/data-models/lib/python3.10/site-packages/schematic/models/validate_attribute.py", line 666, in regex_validation
    validation_rules=self.sg.se.get_class_validation_rules(self.sg.se.get_class_label_from_display_name(manifest_col.name))
  File "/Users/ataylor/miniforge3/envs/data-models/lib/python3.10/site-packages/schematic/schemas/explorer.py", line 432, in get_class_validation_rules
    class_info = self.explore_class(class_label)
  File "/Users/ataylor/miniforge3/envs/data-models/lib/python3.10/site-packages/schematic/schemas/explorer.py", line 333, in explore_class
    if "subClassOf" in self.schema_nx.nodes[schema_class]:
  File "/Users/ataylor/miniforge3/envs/data-models/lib/python3.10/site-packages/networkx/classes/reportviews.py", line 194, in __getitem__
    return self._nodes[n]
KeyError: 'Filename'

I think this actually related to something going on in the JSON-LD of the schema as it contains the following

        {
            "@id": "bts:filename",
            "@type": "rdf:Property",
            "rdfs:comment": "Name of a file",
            "rdfs:label": "filename",
            "schema:domainIncludes": {
                "@id": "bts:File"
            },
            "schema:isPartOf": {
                "@id": "http://schema.biothings.io"
            },
            "sms:displayName": "Filename",
            "sms:required": "sms:true",
            "sms:validationRules": [
               "regex search ^\\S*$"
            ]
        },

Note the lowercase filename for id and label while only displayName has the title case Filename.
Note also that only title case Filename is used in the CSV version of the model

If I manually edit the JSON-LD to have title case Filename for id and label the validation runs as expected, showing the expected error for the second row

        {
            "@id": "bts:Filename",
            "@type": "rdf:Property",
            "rdfs:comment": "Name of a file",
            "rdfs:label": "Filename",
            "schema:domainIncludes": {
                "@id": "bts:File"
            },
            "schema:isPartOf": {
                "@id": "http://schema.biothings.io"
            },
            "sms:displayName": "Filename",
            "sms:required": "sms:true",
            "sms:validationRules": [
               "regex search ^\\S*$"
            ]
        },

(data-models) ataylor@w152 test-nospace-validation % schematic model -c schematic_config.yml validate -mp other_assay_example.csv -dt "OtherAssay"
Starting schematic...
The (model > input > location) argument with value 'HTAN.model.jsonld' is being read from the config file.
The (model > input > file_type) argument with value 'local' is being read from the config file.
JSON schema successfully generated from schema.org schema!
JSON schema file log stored as HTAN.OtherAssay.schema.json
FileDataContext loading zep config
GxConfig.parse_yaml() failed with errors - [{'loc': ('xdatasources',), 'msg': 'field required', 'type': 'value_error.missing'}]
GxConfig.parse_yaml() returning empty `xdatasources`
Loading 'datasources' ->
{}
Loaded 'datasources' ->
{}
EphemeralDataContext has not implemented `_load_zep_config()` returning empty `GxConfig`
Loading 'datasources' ->
{}
Loaded 'datasources' ->
{}
warning: /Users/ataylor/miniforge3/envs/data-models/lib/python3.10/site-packages/great_expectations/data_context/data_context/abstract_data_context.py:2590: DeprecationWarning: create_expectation_suite is deprecated as of v0.15.48 and will be removed in v0.18. Please use add_expectation_suite or add_or_update_expectation_suite instead.
warning:   warnings.warn(
warning: /Users/ataylor/miniforge3/envs/data-models/lib/python3.10/site-packages/great_expectations/data_context/data_context/abstract_data_context.py:373: DeprecationWarning: save_expectation_suite is deprecated as of v0.15.48 and will be removed in v0.18. Please use update_expectation_suite or add_or_update_expectation_suite instead.
warning:   warnings.warn(
        0 expectation(s) included in expectation_suite.
Calculating Metrics: 0it [00:00, ?it/s]
error: For the attribute Filename, on row 3, the string is not properly formatted. It should follow the following re.search pattern "^\S*$".
[['3', 'Filename', 'For the attribute Filename, on row 3, the string is not properly formatted. It should follow the following re.search pattern "^\\S*$".', 'space separated.csv']]

This seems related to this error reported on Slack: https://sagebionetworks.slack.com/archives/C01ANC02U59/p1671129949628629?thread_ts=1671068131.722179&cid=C01ANC02U59

I also wonder if we can't actually validate Filename as it is a Reserve-use special attribute per the Data Model Schema docs.

Does this make sense?

adamjtaylor · 2023-02-22T15:11:21Z

FYI @milen. We would like to add a regex validation rule to prevent spaces in filenames. But we seem to get a key error when adding a regex validation rule to this attribute. See above.

In further testing I can add other validation rules (eg int or str) for the Filename attribute but regex validation rules result in the same error.

adamjtaylor · 2023-02-22T15:31:01Z

Interestingly the schematic example.model.jsonld has titlecase Filename

https://github.com/Sage-Bionetworks/schematic/blob/c9b7ae7178f475effc00a10c70c8548a73422f06/tests/data/example.model.jsonld#LL2305-L2321

brynnz22 · 2023-02-22T17:56:12Z

Yes this makes sense! I saw that Gianna is looking into it on Slack.

GiaJordan · 2023-02-22T18:21:04Z

From slack

@adamjtaylor can you open an issue on this in the schematic repo? I think its more related to the conversion from csv to jsonld. I was able to add the regex rule to our example model and validate without issue. I also noticed that in our model and jsonld, Filename is capitalized. I tried converting the HTAN model with what's on develop and noticed that it was converted as filename like it was in yours. Something aside from the issue with the validation rule is going on and it'll likely take more time to diagnose and solve.

cc @milen-sage @MiekoHash

milen-sage · 2023-03-02T01:53:51Z

Just a note: Filename, should have this capitalization, since schematic treats this attribute specially. It's concerning that it gets lower cased.

adamjtaylor · 2023-03-08T18:24:53Z

Update ahead of the sprint review:

I think this issue is blocked by the lowercasing of Filename in our JSON-LD`
There is a schematic ticket open liked above but it has had no action yet (FYI @milen-sage )
Propose we move this to escalate to monitor resolution
It is a "nice to have" rather than "must have" feature so we can manage waiting
We can also implement this check at the data release stage (@clarisse-lau)

milen-sage · 2023-03-09T16:18:51Z

@adamjtaylor it's in the queue for next sprint triage: see here

MiekoHash · 2023-03-22T15:17:00Z

@adamjtaylor FAIR Data team moved our issue tracking system to JIRA as of this week. Diagnosis of schematic issue is now tracked here.

MiekoHash · 2023-03-27T20:06:07Z

Per Gianna's diagnosis, FAIR's recommendation is that "HTAN should move the Filename specification from the properties column of each attribute to the DependsOn column to resolve"

adamjtaylor · 2023-03-28T07:55:51Z

Thank you @MiekoHash ✨
@aclayton555, @elv-sb I think this competing this move is a separate issue (#195) for us to tackle next data model sprint.

adamjtaylor · 2023-05-09T11:27:24Z

Confirmed in staging that this catches spaces in Filename.

Unfortunately, it also catches Not Applicable in the Channel Metadata filename.

Proposing that we remove the validation from Channel Metadata Filename as we expect deeper validation of these files and a possible transition to synapse IDs.

adamjtaylor · 2023-05-09T11:27:39Z

Confirmed in staging that this catches spaces in Filename.

Unfortunately, it also catches Not Applicable in the Channel Metadata filename.

Proposing that we remove the validation from Channel Metadata Filename as we expect deeper validation of these files and a possible transition to synapse IDs.

aclayton555 assigned adamjtaylor Jan 9, 2023

aclayton555 added the effort-mid This one needs your brain label Jan 9, 2023

brynnz22 mentioned this issue Jan 20, 2023

add regex validation rule to filename attributes #145

Merged

adamjtaylor linked a pull request Jan 23, 2023 that will close this issue

add regex validation rule to filename attributes #145

Merged

adamjtaylor assigned brynnz22 Jan 23, 2023

adamjtaylor mentioned this issue Feb 22, 2023

Key error when regex validation rule in Filename attribute Sage-Bionetworks/schematic#1156

Open

3 tasks

adamjtaylor mentioned this issue Mar 30, 2023

Review where the Properties column is used #195

Closed

aclayton555 unassigned brynnz22 Apr 5, 2023

MiekoHash mentioned this issue Apr 5, 2023

Template name is prefixed by a space ncihtan/HTAN-data-curator#60

Closed

elv-sb closed this as completed in #145 May 8, 2023

adamjtaylor mentioned this issue May 9, 2023

Remove no whitespace validation from Channel Metadata Filename #215

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Enforce no spaces in file names #139

Enforce no spaces in file names #139

adamjtaylor commented Dec 22, 2022

aclayton555 commented Jan 9, 2023

adamjtaylor commented Jan 20, 2023

adamjtaylor commented Jan 20, 2023

adamjtaylor commented Jan 20, 2023

aclayton555 commented Jan 20, 2023

adamjtaylor commented Jan 23, 2023

adamjtaylor commented Jan 23, 2023

adamjtaylor commented Jan 23, 2023

brynnz22 commented Jan 23, 2023 •

edited

Loading

adamjtaylor commented Jan 23, 2023

adamjtaylor commented Feb 22, 2023

adamjtaylor commented Feb 22, 2023

adamjtaylor commented Feb 22, 2023 •

edited

Loading

brynnz22 commented Feb 22, 2023

GiaJordan commented Feb 22, 2023 •

edited

Loading

milen-sage commented Mar 2, 2023

adamjtaylor commented Mar 8, 2023

milen-sage commented Mar 9, 2023

MiekoHash commented Mar 22, 2023

MiekoHash commented Mar 27, 2023

adamjtaylor commented Mar 28, 2023 •

edited

Loading

adamjtaylor commented May 9, 2023

adamjtaylor commented May 9, 2023

Enforce no spaces in file names #139

Enforce no spaces in file names #139

Comments

adamjtaylor commented Dec 22, 2022

aclayton555 commented Jan 9, 2023

adamjtaylor commented Jan 20, 2023

adamjtaylor commented Jan 20, 2023

adamjtaylor commented Jan 20, 2023

aclayton555 commented Jan 20, 2023

adamjtaylor commented Jan 23, 2023

adamjtaylor commented Jan 23, 2023

adamjtaylor commented Jan 23, 2023

brynnz22 commented Jan 23, 2023 • edited Loading

adamjtaylor commented Jan 23, 2023

adamjtaylor commented Feb 22, 2023

adamjtaylor commented Feb 22, 2023

adamjtaylor commented Feb 22, 2023 • edited Loading

brynnz22 commented Feb 22, 2023

GiaJordan commented Feb 22, 2023 • edited Loading

milen-sage commented Mar 2, 2023

adamjtaylor commented Mar 8, 2023

milen-sage commented Mar 9, 2023

MiekoHash commented Mar 22, 2023

MiekoHash commented Mar 27, 2023

adamjtaylor commented Mar 28, 2023 • edited Loading

adamjtaylor commented May 9, 2023

adamjtaylor commented May 9, 2023

brynnz22 commented Jan 23, 2023 •

edited

Loading

adamjtaylor commented Feb 22, 2023 •

edited

Loading

GiaJordan commented Feb 22, 2023 •

edited

Loading

adamjtaylor commented Mar 28, 2023 •

edited

Loading