-
Notifications
You must be signed in to change notification settings - Fork 285
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: plumb Dataset start/end times through graphQL #194
Changes from 3 commits
e50e432
b9014b5
962bdd9
13f3b95
3f6a198
77d9df4
bfe7ed2
cee6796
bfe257c
3859376
1c46785
6b133c5
3db71b6
a47bb20
da3e96b
158135a
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -2,8 +2,8 @@ | |
|
||
from pandas.api.types import is_numeric_dtype, is_object_dtype | ||
|
||
from phoenix.datasets import Dataset | ||
from phoenix.datasets.schema import EmbeddingFeatures | ||
from phoenix.core.datasets import Dataset | ||
from phoenix.core.datasets.schema import EmbeddingFeatures | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think I sorta wanted the code that the user interacts with be unnested and not be part of core. Core being part of the application. I think voxel has a similar organization. Makes for easier discovery in the notebook. But we would dissuade people from importing core in the notebook There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ah ok gotcha -- so datasets is the API interface so to speak, and users should never need to interact with anything in core. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah that was the idea. That the user interacts with datasets and metrics, phoenix serves core via an api. |
||
|
||
from .dimension import Dimension | ||
from .dimension_data_type import DimensionDataType | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,6 +1,21 @@ | ||
from datetime import datetime | ||
from phoenix.core.datasets import Dataset as InternalDataset | ||
import strawberry | ||
|
||
|
||
@strawberry.type | ||
class Dataset: | ||
name: str | ||
start_time: datetime | ||
end_time: datetime | ||
|
||
|
||
def to_gql_dataset(dataset: InternalDataset) -> Dataset: | ||
""" | ||
Converts a phoenix.datasets.Dataset to a phoenix.server.api.types.Dataset | ||
""" | ||
return Dataset( | ||
name=dataset.name, | ||
start_time=dataset.start_time, | ||
end_time=dataset.end_time, | ||
) |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,34 @@ | ||
import pytest | ||
|
||
from phoenix.server.api.types import Dataset | ||
from phoenix.core.datasets import Schema, Dataset as CoreDataset | ||
|
||
from pandas import DataFrame, Timestamp | ||
|
||
|
||
@pytest.fixture | ||
def core_dataset(): | ||
input_df = DataFrame( | ||
{ | ||
"prediction_label": ["apple", "orange", "grape"], | ||
"timestamp": [ | ||
Timestamp(year=2023, month=1, day=1, hour=2, second=30), | ||
Timestamp(year=2023, month=1, day=5, hour=4, second=25), | ||
Timestamp(year=2023, month=1, day=10, hour=6, second=20), | ||
] | ||
} | ||
) | ||
|
||
input_schema = Schema( | ||
prediction_label_column_name="prediction_label", | ||
timestamp_column_name="timestamp", | ||
) | ||
return CoreDataset(dataframe=input_df, schema=input_schema) | ||
|
||
|
||
def test_dataset_serialization(core_dataset): | ||
converted_gql_dataset = Dataset.to_gql_dataset(core_dataset) | ||
|
||
expected_dataset = core_dataset | ||
assert converted_gql_dataset.start_time == expected_dataset.start_time | ||
assert converted_gql_dataset.end_time == expected_dataset.end_time |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's make these non-maybe