shu/ranking pipeline #82
Conversation
wintonzheng
force-pushed
the
shu/ranking_pipeline
branch
from
0b53bf7
to
5f36d64
Compare
|
|
||
|
|
||
| class RankingRequest( | ||
| BaseWyvernRequest, | ||
| PaginationFields, | ||
| Generic[WYVERN_ENTITY], | ||
| CandidateSetEntity, | ||
| Generic[GENERALIZED_WYVERN_ENTITY], |
There was a problem hiding this comment.
Should GENERALIZED_WYVERN_ENTITY be called WYVERN_REQUEST_WITH_ENTITIES? or something like that? The naming here is a bit confusing given that its type can be either a WYVERN_REQUEST or WYVERN_ENTITY
There was a problem hiding this comment.
The generic here is indeed either WYVERN_REQUEST or WYVERN_ENTITY.
There's a plan to make WYVERN_REQUEST a child class of wyvern entity.
| @@ -67,13 +69,16 @@ class RankingResponse(BaseModel): | |||
| events: the list of logged events | |||
| """ | |||
|
|
|||
| ranked_candidates: List[ResponseCandidate] | |||
| ranked_products: List[ResponseCandidate] | |||
There was a problem hiding this comment.
Just curious: Why is this ranked_products? Candidate is a generic term, but product is specific to a type of entity
There was a problem hiding this comment.
Inventa has been using the ranked_products as the schema for all of the use cases. We don't want them to do a API migration on their server side any more. i can also build pydantic "alias" for this field 🤔
There was a problem hiding this comment.
oh. Is it possible to decouple this object from the actual response object? Maybe we can re-name it at the very end or something like that.
They'll definitely want ranked_products or ranked_brands depending on the pipeline.
| ]() | ||
| if not business_logic: | ||
| business_logic = BusinessLogicPipeline[WYVERN_ENTITY, RANKING_REQUEST]() |
There was a problem hiding this comment.
Should this be defaulted in the method signature?
| @@ -228,3 +229,6 @@ async def rank_candidates( | |||
| business_logic_request, | |||
| ) | |||
| return business_logic_response.adjusted_candidates | |||
|
|
|||
| def bypass_ranking(self, request: RankingRequest[WYVERN_ENTITY]) -> bool: | |||
There was a problem hiding this comment.
nit; I would add some documentation here
rename candidate_id back to product_id for ranking piepline response schema
remove query from ranking request
Does this PR have impact on local development experience? If yes, make sure you have a plan and add the documentations to address issues that come with the change
bump version
make a release
publish to pypi service