Data Source factory upgrade #20
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #20 +/- ##
=======================================
Coverage ? 57.84%
=======================================
Files ? 20
Lines ? 1051
Branches ? 0
=======================================
Hits ? 608
Misses ? 443
Partials ? 0
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
Update the DataSource factory - should be a SOFT7Instance factory instead with proper validation of property metadata and more.
Add pytest and pytest-cov dependencies, along with a new `testing` installable extra.
Remove tests for the now re-written SOFT7EntityPropertyType (previously an Enum, now just a Literal collection with an associated mapping).
Satisfy mypy
Also, rename create_entity() to create_datasource(). Ensure the create_datasource() function returns an instantiated model.
This can now be used in both the SOFT7DataSource model as well as the new DataSourceDimensions model. Add `requests-mock` dependency. Consider using "python" backend for OTELib instead to avoid mocking requests. Remove all filterwarnings from pytest config.
New test using python otelib backend.
Added test for serializing
All the serialization tests (including JSON Schema) must be extended to ensure not just that it _can_ be serialized, but that the content is also what would be expected.
Implemented as a pydantic model_serializer in "wrap" mode. Temporarily changed the otelib dependency to point to a git branch on GitHub that implements migration to pydantic v2.
Make the namespace an AnyUrl.
Quick (but proper) solutions to ever so slightly decrease the number of uncovered lines.
CasperWA
force-pushed
the
cwa/soft7-instance-factory
branch
from
16893aa
to
a1b2ac8
Compare
CasperWA
changed the title
Specifically, move all the "helper"/utility functionalities from `datasource_factory.py` to a new `soft7_instance.py` under pydantic_models.
Run hooks on all files, upgrading them accordingly.
Make factory functions importable from `s7.factories` as `create_*`, where * matches the name of any `*_factory.py` file in the folder.
Split ruff hook for core code base code and non-core.
Started to flesh out an OTEAPI function strategy plugin for converting between a parsed data source to a SOFT7 Entity instance.
Missing finalizing example in notebook.
There is a design issue here with nested entities that should be fixable in a more elegant way
Need to fix setting ClassVar on EntityInstance and the validator for the same.
Add a YAML parse strategy for OTEAPI.
See issue #395 in oteapi-core to follow up on when this ignore statement can be removed.
1 task
|
Closed in favor of #36 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
DO NOT MERGE until #16 has been merged and this has been updated accordingly, since this PR branch is based on the HEAD of the #16 PR branch.One-sentence overview
This PR does a few different things, all centered around the
s7.factories.datasource_factorymodule.Detailed overview
Similarly to #17 (it even cherry-picks a commit from this PR branch) it introduces testing through the
pytestframework.The tests are mainly for the
create_entity()factory function (now renamed tocreate_datasource()) and the SOFT7 data source instance it returns.Note, some negative-outcome tests are still missing (i.e., testing exceptions are raised correctly when expected).
A few liberties have been taken with regards to the resulting scheme of the data source instance. The
s7.pydantic_models.softmodule has been updated to accommodate and represent the result of these.The point of upgrading this particular module is to implement validators and serializers (using the new features of pydantic v2) to ensure a data source instance respects the derived logic from its SOFT7 Entity. The most important logic not respected/validated was the
property.shapetodimensionsrelation.Summary
Essentially, the result of this PR is that at this point, the SOFT7 Data Source can be considered the archetypal SOFT7 "instance", i.e., a data structure described by a SOFT7 Entity.
PR Status
The main upgrade has been implemented.
There are still tests missing, especially for testing expected failures, but also for testing more intricate data models, e.g., properties with a multi-dimensional shape.
Finally, a restructuring of the code should be performed to more easily read the code (and the important bits like
create_datasource()and_get_data()).Especially for this last part,
_get_data()still needs some upgrading according to how we intend to retrieve properties within a DataSpace. It needs flexibility and possibly "levels of laziness". In other words, currently_get_data()uses OTEAPI to retrieve and cache the data for a model locally, which is then inspected lazily when requested. I suspect the whole OTEAPI Services call workflow should be part of the lazy functionality, and not cached - but it might be desirable to indeed cache it sometimes..._get_data()function (possibly in a separate PR)