[SYNPY-1681] Set JSON Schema type based on columnType; remove type setting via validation rules #1274
Conversation
| return model_dataframe | ||
|
|
||
|
|
||
| class MetadataModel(object): |
There was a problem hiding this comment.
This is a pared-down version of schematic’s MetadataModel. It omits legacy features—such as manifest validation and submission—that we no longer need. See the original MetadataModel for reference: https://github.com/Sage-Bionetworks/schematic/blob/develop/schematic/models/metadata.py#L28
|
|
||
| def __init__( | ||
| self, | ||
| inputMModelLocation: str, |
There was a problem hiding this comment.
this is not up to the variable standard of the synapse python client. I will either adjust it in this PR or create a new ticket.
linglp
changed the title
.github/workflows/build.yml
Outdated
| ${{ steps.get-dependencies.outputs.site_packages_loc }} | ||
| ${{ steps.get-dependencies.outputs.site_bin_dir }} | ||
| key: ${{ runner.os }}-${{ matrix.python }}-build-${{ env.cache-name }}-${{ hashFiles('setup.py') }}-v28 | ||
| key: ${{ runner.os }}-${{ matrix.python }}-build-${{ env.cache-name }}-${{ hashFiles('setup.py', 'setup.cfg') }}-v28 |
There was a problem hiding this comment.
Make sure that CI/CD reinstall dependencies if setup.cfg changes.
| return json_schema_dict | ||
|
|
||
|
|
||
| def _write_data_model( |
There was a problem hiding this comment.
this is originally in schematic when calling create_json_schema. Please see the following for reference:
| May raise errors if the component is not found in the data model graph. | ||
| """ | ||
|
|
||
| metadata_model = MetadataModel( |
There was a problem hiding this comment.
This is part of the original functionality in schematic: https://github.com/Sage-Bionetworks/schematic/blob/develop/schematic/schemas/json_schema_component_generator.py#L245
| datatype=self.component, | ||
| logger=self.logger, | ||
| schema_name=self.component + "_validation", | ||
| jsonld_path=metadata_model.inputMModelLocation, |
There was a problem hiding this comment.
this is part of the original function in schematic: https://github.com/Sage-Bionetworks/schematic/blob/develop/schematic/schemas/json_schema_component_generator.py#L257
| ) | ||
|
|
||
| TESTS_DIR = os.path.dirname(os.path.abspath(__file__)) | ||
| SCHEMA_FILES_DIR = os.path.join(TESTS_DIR, "schema_files") |
There was a problem hiding this comment.
There's no more data directory. I organized everything under the new schema_files directory
| TEST_DISPLAY_NAMES_SCHEMA_PATTERN = "test.{datatype}.display_names_schema.json" | ||
|
|
||
|
|
||
| # Helper functions for path construction |
There was a problem hiding this comment.
The original schematic has file paths hard-coded all over all the place in the test. Please see below as some examples:
- https://github.com/Sage-Bionetworks/schematic/blob/develop/tests/unit/test_create_json_schema.py#L665
- https://github.com/Sage-Bionetworks/schematic/blob/develop/tests/unit/test_create_json_schema.py#L697
All these hard-coded file paths make maintenance difficult when the file locations change. I changed it so that they are all organized using these helper functions.
thomasyu888
left a comment
thomasyu888
left a comment
There was a problem hiding this comment.
I know this is draft mode, but you may want to consider letting the team know to work off of this branch when ready as there are some big changes here that may or may not impact people's development of curator extension.
| INT = "int" | ||
| BOOL = "bool" | ||
| NUM = "num" | ||
|
|
There was a problem hiding this comment.
Remove the type validation rules here
|
Thanks @linglp ! LGTM |
| ${{ steps.get-dependencies.outputs.site_packages_loc }} | ||
| ${{ steps.get-dependencies.outputs.site_bin_dir }} | ||
| key: ${{ runner.os }}-${{ matrix.python }}-build-${{ env.cache-name }}-${{ hashFiles('setup.py') }}-v28 | ||
| key: ${{ runner.os }}-${{ matrix.python }}-build-${{ env.cache-name }}-${{ hashFiles('setup.py') }}-v29 |
There was a problem hiding this comment.
I updated it to v29 to ensure dependencies can be updated when running github actions.
Problem:
Solution:
_write_data_model: https://github.com/Sage-Bionetworks/schematic/blob/develop/schematic/schemas/create_json_schema.py#L658conftestfrom schematic: https://github.com/Sage-Bionetworks/schematic/blob/develop/tests/conftest.py.MetadataModelcreate_json_schematest_create_json_schema.pyschema_filesdirectoryThe following functions are now removed:
get_js_type_from_inputted_rulesThe following functions will be deprecated:
Testing:
dmgefixture now uses example.model.column_type_component.csv from this referencefixture_test_directorynow uses Pytest’s built-in temporary directory, as shown here (instead of the original implementation). This eliminates the need for custom cleanup logic under the now-deprecated data directory.