Skip to content
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

Remove FeatureViewProjections & handle its responsibility with FeatureView and FeatureService #1907

Closed
mavysavydav opened this issue Sep 23, 2021 · 0 comments · Fixed by #1899
Assignees

Comments

@mavysavydav
Copy link
Collaborator

We'd like to enhance FeatureView with methods like ".with_name" (#1867) and potentially ".with_entities" as part as this shadow entities mapping RFC -- https://docs.google.com/document/d/1TsCwKf3nVXTAfL0f8i26jnCgHA3bRd4dKQ8QdM87vIA/edit#heading=h.o7edalv7g2mz.

The trouble right now is that at query time, even though the FV is one concept in the users mind when use get_historical_features or get_online_features, but it actually splits into a different class when the getitem magic method is triggered by my_fv[[<my-feature-selections]] and it becomes a FVProjection. This means if users wanted to transform their FV, we'd have to support these methods on both the FV class and FVProjection class.

FVProjection is only used for 1 second when FV[[..]] is passed in FeatureService. After that, everything goes back to feature_ref strings and FeatureViews. In a world in which we make FVProjection carry more data/logic to support the needed upgrades for shadow entities mapping, it would require a big refactor if we wanted to carry the needed mapping info in FVProjection and have all the downstream code accept FVProjection. Now I think even tho FVProjection may have value in the future depending on what additional feast product features we add in the future, rn the state of Feast is a lot better suited for having FV be the sole grouping object (rather than having FVProjection as well). I think once having FVProjection proves its value, we can make the changes to make the switch.

In terms of risk management, the approach of eliminating FVProjection wouldn’t require much code changes and if we were to change our mind later on, then we can do the big refactoring. This is as opposed to doing the big refactoring first to string FVProjections through everything and then if we change our minds, we have to do another big refactoring to get back to where we are now. So worse case scenarios are 1 big refactoring for the former whereas 2 big refactorings for latter.

We must take care to be very intentional about what new functionality is added to FV and FeatureService and the bounds of these additional responsibilities must be clear.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants