SchemaView object should be instantiated when needed & not globally.
#323
Conversation
hrshdhgd
changed the title
schema.pyschema.py
hrshdhgd
changed the title
schema.pySchemaView object should be instantiated when needed & not globally.
| @@ -628,7 +627,7 @@ def decorator(f): | |||
| @main.command() | |||
| @input_argument | |||
| @output_option | |||
| @dynamically_generate_sssom_options(MAPPING_SLOTS) | |||
| @dynamically_generate_sssom_options(SSSOMSchemaView().mapping_slots) | |||
There was a problem hiding this comment.
I wonder if this will defeat the purpose of the refactoring. I should have thought of this before.
I am wondering now if we need to find a way to skip network access in SchemaView()?
sssom/constants.py
Outdated
|
|
||
| entity_reference = "EntityReference" | ||
| yaml = pkg_resources.resource_filename("sssom_schema", "schema/sssom_schema.yaml") | ||
| view = SchemaView(yaml) |
There was a problem hiding this comment.
I am not sure, but you may have to use init() to avoid this code being run whenever the class is referenced:
https://stackoverflow.com/questions/9056957/correct-way-to-define-class-variables-in-python
But not 100% sure.
sssom/util.py
Outdated
| @@ -1040,6 +1036,18 @@ class NoCURIEException(ValueError): | |||
|
|
|||
|
|
|||
| CURIE_RE = re.compile(r"[A-Za-z0-9_.]+[:][A-Za-z0-9_]") | |||
| schema_view_object = SSSOMSchemaView() | |||
There was a problem hiding this comment.
this is again in the global context I think.. wont this be invoked any time anyone imports anything from utils package?
There was a problem hiding this comment.
I will have to repeat this code in 4 different lines in the same file. Seems messy.
| @@ -284,7 +284,7 @@ def to_rdf_graph(msdf: MappingSetDataFrame) -> Graph: | |||
| # os.remove("sssom.ttl") # remove the intermediate file. | |||
| graph = rdflib_dumper.as_rdf_graph( | |||
| element=doc.mapping_set, | |||
| schemaview=SchemaView(SCHEMA_YAML), | |||
| schemaview=SchemaView(SSSOMSchemaView().yaml), | |||
There was a problem hiding this comment.
This is a bit weird... :D
| def view(self) -> SchemaView: | ||
| """Return SchemaView object.""" | ||
| if self._view is None: | ||
| self._view = SchemaView(self.yaml) |
There was a problem hiding this comment.
smart, so, whenever self.view is accessed as a variable, this function is executed, effectively deferring the SchemaView() instantiation to the first time its executed? Cool!
|
|
||
| def test_slots(self) -> None: | ||
| """Test slots.""" | ||
| self.assertEqual(len(self.sv.mapping_slots), 39) |
There was a problem hiding this comment.
probably better test self.assertTrue(len(self.sv.mapping_slots)>=39) because changes to the metadata model will break this test every time?
There was a problem hiding this comment.
Good call, changed!
| @@ -52,7 +52,7 @@ description = Run the flake8 code quality checker. | |||
| [testenv:mypy] | |||
| deps = mypy | |||
| skip_install = true | |||
| commands = mypy --install-types --non-interactive --ignore-missing-imports sssom/ setup.py | |||
| commands = mypy --install-types --non-interactive --ignore-missing-imports --implicit-optional sssom/ setup.py | |||
There was a problem hiding this comment.
In its absence mypy throws a bunch of errors.
There was a problem hiding this comment.
nothing you think is serious enough to be fixed though? I will trust you on this, just doing due diligence
| @@ -19,6 +19,8 @@ def test_sort(self): | |||
| """Test sorting of columns.""" | |||
| new_df = sort_df_rows_columns(self.msdf.df) | |||
| column_sequence = [ | |||
| col for col in SCHEMA_DICT["slots"].keys() if col in new_df.columns | |||
| col | |||
| for col in SSSOMSchemaView().dict["slots"].keys() | |||
There was a problem hiding this comment.
so weird SchemaView() has no "get_slots()".
|
See #324 |
Fixes #322
This is because it invokes a network call when
sssom-pyis imported. This is unnecessary.