-
Notifications
You must be signed in to change notification settings - Fork 518
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
Move transformations from DataSet
to DataSetFilters
#6799
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #6799 +/- ##
==========================================
- Coverage 97.50% 97.50% -0.01%
==========================================
Files 143 143
Lines 28188 28187 -1
==========================================
- Hits 27485 27484 -1
Misses 703 703
|
Related issue on decoupling filters from datasets: |
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.
Agree with the change. LGTM.
Just a heads up that since a few doc build images were removed by this PR, this messes up the build cache. So, all Documentation Build checks will now fail unless they use the |
Overview
We have
DataSetFilters.transform
andDataSetFilters.reflect
, but all other transformation methods areDataSet
methods. This is inconsistent and confusing, especially since these methods pretty much all callDataSetFilters.transform
internally.This PR moves all methods to
DataSetFilters
. Related tests are also moved.