Transform derived integration and derived validation - #61
Conversation
…ved transform query validation ref: AB#21929
…rt' and 'import from'' Co-authored-by: Copilot Autofix powered by AI <223894421+github-code-quality[bot]@users.noreply.github.com>
…om/ARKlab/Artesian.SDK-Python into 21929-TransformDerivedAndQueryTest
Co-authored-by: Copilot Autofix powered by AI <223894421+github-code-quality[bot]@users.noreply.github.com>
…om/ARKlab/Artesian.SDK-Python into 21929-TransformDerivedAndQueryTest
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 13 out of 13 changed files in this pull request and generated no new comments.
Comments suppressed due to low confidence (4)
README.md:719
- README example imports MarketDataType from the top-level Artesian package, but MarketDataType is exported from Artesian.MarketData (not Artesian.init). As written,
from Artesian import MarketDataTypewill fail.
from Artesian import MarketDataType
README.md:729
- README example passes
rowsas a list of tuples, but TimeSerieData.rows is a Dict[datetime, float] and the SDK serializer expects dict-like input to produce the {Key,Value} list shape. The example should use a dict keyed by datetime.
rows=[
(datetime(2018, 10, 1, 0, 0), 100),
(datetime(2018, 10, 1, 1, 0), 100)
],
src/Artesian/MarketData/_Dto/DerivedCfg.py:16
- DerivedCfg now supports the Transform algorithm, but the docstring Attributes section doesn’t document the new
transformfield, which makes the public API harder to understand.
derivedAlgorithm: the derived configuration algorithm (MUV, Coalesce, Sum, Transform)
version: the derived configuration version
orderedReferencedMarketDataIds: the ordered reference MarketData Ids
used in the computation
src/Artesian/MarketData/_Dto/DerivedTransformQueryValidation.py:33
- DerivedTransformQueryValidation treats
transformas required (it raises in post_init), but the type is declared Optional[str]. This makes type hints misleading and forces runtime validation for something that can be enforced by the signature.
transform: Optional[str] = None
def __post_init__(self: "DerivedTransformQueryValidation") -> None:
if self.transform is None:
raise ValueError("transform must be provided for query validation.")
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 13 out of 14 changed files in this pull request and generated 1 comment.
Comments suppressed due to low confidence (5)
README.md:719
- The README example imports
MarketDataTypefrom the top-levelArtesianpackage, butArtesian/__init__.pydoes not export it. This snippet will raise ImportError; importMarketDataTypefromArtesian.MarketDatainstead.
from Artesian import MarketDataType
README.md:729
TimeSerieData.rowsis aDict[datetime, Optional[float]], but the README example passes a list of tuples. This won’t serialize to the expected[{Key, Value}, ...]payload used by the SDK; use a dict like other time-series examples in the README.
rows=[
(datetime(2018, 10, 1, 0, 0), 100),
(datetime(2018, 10, 1, 1, 0), 100)
],
src/Artesian/MarketData/_Dto/DerivedTransformQueryValidation.py:33
transformis required (enforced by__post_init__), but it is typed asOptional[str]with a default ofNone. This makes the public API/type hints inaccurate; make it a requiredstrand drop the runtime check.
transform: Optional[str] = None
def __post_init__(self: "DerivedTransformQueryValidation") -> None:
if self.transform is None:
raise ValueError("transform must be provided for query validation.")
src/Artesian/MarketData/_Dto/init.py:5
- New DTOs for derived transform validation are not re-exported from
Artesian.MarketData._Dto, while other DTOs are. This makesfrom Artesian.MarketData._Dto import DerivedTransformQueryValidationfail and is inconsistent with the module’s public surface; add the missing imports here.
from .MarketDataEntityInput import MarketDataEntityInput
from .MarketDataEntityOutput import MarketDataEntityOutput
from .CheckConversionResult import CheckConversionResult
from .UnitOfMeasure import UnitOfMeasure
from .TimeSerieData import TimeSerieData
src/Artesian/MarketData/_Dto/init.py:40
__all__should include the newly added derived transform validation DTOs so they are importable fromArtesian.MarketData._Dtolike the other DTOs.
DerivedCfg.__name__,
CheckConversionResult.__name__,
UnitOfMeasure.__name__,
TimeSerieData.__name__,
] # type: ignore
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 13 out of 14 changed files in this pull request and generated 1 comment.
Comments suppressed due to low confidence (2)
README.md:720
- This example imports MarketDataType from the top-level Artesian package, but MarketDataType is exported from Artesian.MarketData (not Artesian.init). The current import will fail for users copy/pasting the snippet.
from Artesian.MarketData import MarketDataService
from Artesian.MarketData._Dto.DerivedTransformQueryValidation import DerivedTransformQueryValidation
from Artesian.MarketData._Dto.TimeSerieData import TimeSerieData
from Artesian import MarketDataType
from datetime import datetime
README.md:730
- TimeSerieData.rows is a dict[datetime, float|None] in the SDK and is serialized as a list of {Key, Value} items. This example uses a list of tuples, which doesn't match the DTO shape and won't serialize to the expected payload.
data=TimeSerieData(
rows=[
(datetime(2018, 10, 1, 0, 0), 100),
(datetime(2018, 10, 1, 1, 0), 100)
],
type=MarketDataType.ActualTimeSerie,
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 13 out of 14 changed files in this pull request and generated no new comments.
Comments suppressed due to low confidence (5)
README.md:719
- README example imports MarketDataType from the top-level Artesian package, but MarketDataType is exported from Artesian.MarketData. The current import in the docs will raise ImportError for users following the example.
from Artesian import MarketDataType
README.md:730
- README example passes TimeSerieData.rows as a list of (datetime, value) tuples, but TimeSerieData.rows is defined as a Dict[datetime, float]. The example as written won’t match the SDK type hints/serialization (Dict fields serialize as a list of {Key, Value} objects).
rows=[
(datetime(2018, 10, 1, 0, 0), 100),
(datetime(2018, 10, 1, 1, 0), 100)
],
type=MarketDataType.ActualTimeSerie,
src/Artesian/MarketData/_Dto/DerivedCfg.py:16
- DerivedCfg docstring was updated to mention the new Transform algorithm, but it doesn’t document the new
transformfield that was added to the dataclass. This leaves the public docs for DerivedCfg incomplete/inconsistent with the actual API surface.
derivedAlgorithm: the derived configuration algorithm (MUV, Coalesce, Sum, Transform)
version: the derived configuration version
orderedReferencedMarketDataIds: the ordered reference MarketData Ids
used in the computation
src/Artesian/MarketData/_Dto/init.py:6
- New DTOs DerivedTransformQueryValidation / DerivedTransformQueryValidationResponse are added in this PR but not re-exported from Artesian.MarketData._Dto. This makes
from Artesian.MarketData._Dto import ...inconsistent with other DTOs and harder to discover/use.
from .TimeSerieData import TimeSerieData
from .CurveRangeEntity import CurveRangeEntity
src/Artesian/MarketData/_Dto/init.py:40
- These new DTOs should also be added to all so wildcard imports and tooling (and any code relying on all) can see them, consistent with the other DTOs listed here.
DerivedCfg.__name__,
CheckConversionResult.__name__,
UnitOfMeasure.__name__,
TimeSerieData.__name__,
] # type: ignore
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 13 out of 14 changed files in this pull request and generated no new comments.
Comments suppressed due to low confidence (2)
README.md:719
- The README example imports
MarketDataTypefrom the rootArtesianpackage, butMarketDataTypeis exported fromArtesian.MarketData(notArtesian). This example will raiseImportErrorfor users following the docs.
from Artesian import MarketDataType
README.md:729
TimeSerieData.rowsis defined/used as a dict keyed by datetime (serialized as a list of{Key, Value}pairs). The README example showsrowsas a list of tuples, which won’t serialize to the expected request body forDerivedTransformQueryValidation.
data=TimeSerieData(
rows=[
(datetime(2018, 10, 1, 0, 0), 100),
(datetime(2018, 10, 1, 1, 0), 100)
],
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 13 out of 14 changed files in this pull request and generated no new comments.
Comments suppressed due to low confidence (4)
README.md:720
- README snippet imports
MarketDataTypefromArtesian, butArtesian/__init__.pydoes not export it. This example will fail withImportError. Import it fromArtesian.MarketData(or useArtesian.MarketData.MarketDataType).
from Artesian.MarketData import MarketDataService
from Artesian.MarketData._Dto.DerivedTransformQueryValidation import DerivedTransformQueryValidation
from Artesian.MarketData._Dto.TimeSerieData import TimeSerieData
from Artesian import MarketDataType
from datetime import datetime
README.md:731
TimeSerieData.rowsis defined/serialized as a dict ofdatetime -> float(serialized to a list of{Key, Value}objects). The README example passes a list of tuples, which won’t serialize to the expected request shape.
data=TimeSerieData(
rows=[
(datetime(2018, 10, 1, 0, 0), 100),
(datetime(2018, 10, 1, 1, 0), 100)
],
type=MarketDataType.ActualTimeSerie,
),
src/Artesian/MarketData/_Dto/init.py:6
- New DTOs for derived transform query validation are added in this PR, but
_Dto/__init__.pydoesn’t re-export them. This forces deep imports and is inconsistent with how other DTOs (e.g.DeleteData) are exposed.
from .MarketDataEntityInput import MarketDataEntityInput
from .MarketDataEntityOutput import MarketDataEntityOutput
from .CheckConversionResult import CheckConversionResult
from .UnitOfMeasure import UnitOfMeasure
from .TimeSerieData import TimeSerieData
from .CurveRangeEntity import CurveRangeEntity
src/Artesian/MarketData/_Dto/init.py:40
_Dto/__init__.pyshould include the new derived transform validation DTOs in__all__sofrom Artesian.MarketData._Dto import DerivedTransformQueryValidationworks like other DTOs.
__all__ = [
MarketDataEntityOutput.__name__,
MarketDataEntityInput.__name__,
CurveRangeEntity.__name__,
PagedResultCurveRangeEntity.__name__,
MarketDataIdentifier.__name__,
AuctionBidValue.__name__,
AuctionBids.__name__,
BidAskValue.__name__,
MarketAssessmentValue.__name__,
UpsertData.__name__,
DeleteData.__name__,
ArtesianSearchResults.__name__,
ArtesianMetadataFacet.__name__,
ArtesianMetadataFacetCount.__name__,
DerivedCfg.__name__,
CheckConversionResult.__name__,
UnitOfMeasure.__name__,
TimeSerieData.__name__,
] # type: ignore
transform derived integration and derived validation
ref: AB#21929