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.

Sign up for GitHub

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

Jump to bottom

clean up after pre-release-cleanup #537

Merged
merged 67 commits into from
Jan 30, 2024
Merged

clean up after pre-release-cleanup #537

merged 67 commits into from
Jan 30, 2024

Conversation

chlebowa
Copy link
Contributor

@chlebowa chlebowa commented Jan 26, 2024 •
edited
Loading

Proof reading documentation and code comments.

Aleksander Chlebowski added 30 commits January 26, 2024 11:29
fix FilteredData instantiation
eed5e92
update docs for FilterState-utils
da964be
update docs for FilterState
a37f60e
update docs for FilterState
9cbcaa6
update docs for FilterState
a49cf53
update docs for ChoicesFilterState
ce1d380
update docs for ChoicesFilterState
cba6202
update docs for DateFilterState
b8daaa8
update docs for DatetimeFilterState
027663f
update docs for EmptyFilterState
d7a3533
update docs for FilterStateExpr
8f6b4b7
update docs for LogicalFilterState
474cfd5
update docs for RangeFilterState
ebef7e4
minor update
cd3893a
add return value description to init on FilterState daughters
da194fa
fix documentation for FilterStates utils
144a383
filtering -> subsetting
b6e4083
tweak Description in DESCRIPTION
2b3fae6
remove mention of na_rm (old relic)
84eb327
subsetting call -> condition call
d095acf
clean up FilterStates
14120fa
fix docs for DFFilterStates
c04baea
fix docs for MAEFilterStates
3d26fa3
fix docs for MatrixFilterStates
e65322d
fix docs for SEFilterStates
6da3c2d
update docs for FilteredData utils
852f75a
update topological_sort
6bfb0f2
re-fix topological_sort
ef4db39
fix documentation ofr FilteredData
5242940
update documentation ofr FilteredData
d5e69b8
@averissimo
Copy link
Contributor

Looks good to merge as soon as those # TODO tags are addressed (or discarded)

As well as the other reviewer's comments

pawelru
pawelru reviewed Jan 30, 2024
View reviewed changes
R/FilterState.R Outdated Show resolved Hide resolved
pawelru
pawelru reviewed Jan 30, 2024
View reviewed changes
R/FilterState.R Outdated Show resolved Hide resolved
pawelru
pawelru reviewed Jan 30, 2024
View reviewed changes
R/FilterStateChoices.R Outdated Show resolved Hide resolved
pawelru
pawelru reviewed Jan 30, 2024
View reviewed changes
R/FilterStateChoices.R Outdated Show resolved Hide resolved
pawelru
pawelru reviewed Jan 30, 2024
View reviewed changes
R/FilterStateRange.R Outdated Show resolved Hide resolved
chlebowa and others added 3 commits January 30, 2024 12:28
@chlebowa @pawelru
missing period
715a717 
Co-authored-by: Pawel Rucki <[email protected]>
Signed-off-by: Aleksander Chlebowski <[email protected]>
@chlebowa @pawelru
typo
cee1fa6 
Co-authored-by: Pawel Rucki <[email protected]>
Signed-off-by: Aleksander Chlebowski <[email protected]>
@chlebowa @pawelru
typo
b176a06 
Co-authored-by: Pawel Rucki <[email protected]>
Signed-off-by: Aleksander Chlebowski <[email protected]>
pawelru
pawelru reviewed Jan 30, 2024
View reviewed changes
R/FilteredDataset-utils.R Outdated Show resolved Hide resolved
pawelru
pawelru reviewed Jan 30, 2024
View reviewed changes
R/FilteredDataset.R Outdated Show resolved Hide resolved
pawelru
pawelru reviewed Jan 30, 2024
View reviewed changes
R/choices_labeled.R Outdated Show resolved Hide resolved
pawelru
pawelru approved these changes Jan 30, 2024
View reviewed changes
Copy link
Contributor

@pawelru pawelru left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This all looks good and must have been a lot of work to go through this. Thank you 👍 I left few comments but none of them are super strong.

@github-actions GitHub Actions
Copy link
Contributor

github-actions bot commented Jan 30, 2024 •
edited
Loading

Unit Test Performance Difference

Additional test case details
Test Suite $Status$ Time on main $±Time$ Test Case
MAEFilteredDataset 💀 $0.40$ $-0.40$ get_filter_overview_info_returns_overview_matrix_for_MAEFilteredDataset_with_filtering
MAEFilteredDataset 💀 $0.09$ $-0.09$ get_filter_overview_info_returns_overview_matrix_for_MAEFilteredDataset_without_filtering
MAEFilteredDataset 👶 $+0.39$ get_filter_overview_returns_overview_matrix_for_MAEFilteredDataset_with_filtering
MAEFilteredDataset 👶 $+0.09$ get_filter_overview_returns_overview_matrix_for_MAEFilteredDataset_without_filtering

Results for commit 49574cb

♻️ This comment has been updated with latest results.

@chlebowa chlebowa force-pushed the post-pre-release-cleanup-cleanup@main branch from fd20572 to 17c3550 Compare January 30, 2024 14:38
@donyunardi donyunardi mentioned this pull request Jan 30, 2024
Closed
34 tasks
Aleksander Chlebowski and others added 3 commits January 30, 2024 16:10
remove more TODOs
6803421
update note about init_filter_states
e9e2e9a
@chlebowa
507 reevaluate variable_types (#508)
82c8406 
Closes #507 

Replaced `variable_types` function family with single function.
The new function returns a named vector so no need to use `setNames` in
its caller.
@chlebowa chlebowa merged commit 37c351d into main Jan 30, 2024
24 checks passed
@chlebowa chlebowa deleted the post-pre-release-cleanup-cleanup@main branch January 30, 2024 16:40
@chlebowa
Copy link
Contributor Author

Thank you for lending a hand @pawelru @averissimo 💪

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@averissimo averissimo averissimo left review comments

@pawelru pawelru pawelru approved these changes

Assignees

@averissimo averissimo

Labels
core
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@chlebowa @averissimo @pawelru