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

Change qenv as environment "type" -- adds names(qenv/qenv.error), get() and $ S3 methods #218

Merged
merged 58 commits into from
Nov 8, 2024

Conversation

averissimo
Copy link
Contributor

@averissimo averissimo commented Oct 28, 2024

Pull Request

Changes description

  • qenv S4 class inherits from environment data class
  • Removes @env slot in favor of qenv
  • Replace all instances of @env with @.xData (slot created by parent class)
  • All functions/methods that work for environment class are supported natively in qenv

@averissimo averissimo marked this pull request as ready for review October 28, 2024 17:09
Copy link
Contributor

github-actions bot commented Oct 28, 2024

Unit Tests Summary

  1 files   10 suites   1s ⏱️
124 tests 122 ✅ 2 💤 0 ❌
219 runs  217 ✅ 2 💤 0 ❌

Results for commit a5fcb9a.

♻️ This comment has been updated with latest results.

Copy link
Contributor

github-actions bot commented Oct 28, 2024

Unit Test Performance Difference

Test Suite $Status$ Time on main $±Time$ $±Tests$ $±Skipped$ $±Failures$ $±Errors$
qenv-class 👶 $+0.25$ $+7$ $0$ $0$ $0$
Additional test case details
Test Suite $Status$ Time on main $±Time$ Test Case
qenv-class 👶 $+0.10$ creates_a_locked_environment
qenv-class 👶 $+0.03$ creates_a_locked_environment_when_.xData_is_manually_defined
qenv-class 👶 $+0.01$ initialized_qenv_s_have_different_environments
qenv-class 👶 $+0.01$ throws_error_when_.xData_is_not_an_environment
qenv-class 👶 $+0.01$ throws_error_when_code_is_not_language_or_character_object
qenv-class 👶 $+0.11$ throws_error_when_id_and_code_length_doesn_t_match
qenv_constructor 👶 $+0.01$ does_not_allow_binding_to_be_added
qenv_constructor 👶 $+0.01$ does_not_allow_binding_to_be_modified
qenv_constructor 👶 $+0.02$ is_an_environment
qenv_constructor 👶 $+0.00$ ls_all.names_TRUE_show_all_objects
qenv_constructor 👶 $+0.00$ ls_does_not_show_hidden_objects
qenv_constructor 👶 $+0.00$ names_shows_available_objets
qenv_constructor 👶 $+0.00$ names_shows_hidden_objects
qenv_constructor 👶 $+0.00$ names_shows_nothing_on_empty_environment
qenv_constructor 💀 $0.01$ $-0.01$ parent_of_qenv_environment_is_locked
qenv_constructor 💀 $0.00$ $-0.00$ parent_of_qenv_environment_is_the_parent_of_.GlobalEnv
qenv_constructor 👶 $+0.00$ via_qenv_directly
qenv_constructor 👶 $+0.00$ via_slot
qenv_eval_code 💀 $0.01$ $-0.01$ env_in_qenv_is_always_a_sibling_of.GlobalEnv
qenv_get_code 💀 $0.01$ $-0.01$ works_for_datanames_of_length_1
qenv_get_code 👶 $+0.01$ works_for_names_of_length_1
qenv_get_var 👶 $+0.01$ get_var_and_only_returns_objects_from_qenv_not_.GlobalEnv
qenv_within 👶 $+0.00$ multiline_expressions_are_evaluated
qenv_within 👶 $+0.01$ within.qenv_renturns_a_qenv_where_.xData_is_a_deep_copy_of_that_in_data_
qenv_within 💀 $0.01$ $-0.01$ within.qenv_renturns_a_qenv_where_env_is_a_deep_copy_of_that_in_data_

Results for commit eecbc6d

♻️ This comment has been updated with latest results.

Copy link
Contributor

github-actions bot commented Oct 28, 2024

badge

Code Coverage Summary

Filename                         Stmts    Miss  Cover    Missing
-----------------------------  -------  ------  -------  ---------
R/qenv-c.R                          57       2  96.49%   75, 106
R/qenv-class.R                      19       2  89.47%   42, 45
R/qenv-concat.R                     10       0  100.00%
R/qenv-constructor.R                 1       0  100.00%
R/qenv-errors.R                      4       4  0.00%    6-9
R/qenv-eval_code.R                  52       2  96.15%   99, 108
R/qenv-get_code.R                   28       0  100.00%
R/qenv-get_env.R                     3       3  0.00%    22-27
R/qenv-get_var.R                    27       0  100.00%
R/qenv-get_warnings.R               24       0  100.00%
R/qenv-join.R                        7       0  100.00%
R/qenv-length.R                      2       0  100.00%
R/qenv-show.R                        1       1  0.00%    19
R/qenv-within.R                      8       0  100.00%
R/utils-get_code_dependency.R      191       1  99.48%   283
R/utils.R                            9       0  100.00%
TOTAL                              443      15  96.61%

Diff against main

Filename                Stmts    Miss  Cover
--------------------  -------  ------  --------
R/qenv-c.R                +57      +2  +96.49%
R/qenv-class.R            +19      +2  +89.47%
R/qenv-constructor.R      -15     -13  +81.25%
R/qenv-errors.R            +4      +4  +100.00%
R/qenv-get_var.R           +8       0  +100.00%
R/qenv-join.R             -39       0  +100.00%
R/qenv-length.R            +2       0  +100.00%
R/utils.R                  -1       0  +100.00%
TOTAL                     +35      -5  +1.52%

Results for commit: a5fcb9a

Minimum allowed coverage is 80%

♻️ This comment has been updated with latest results

@gogonzo gogonzo self-assigned this Oct 29, 2024
R/qenv-names.R Outdated Show resolved Hide resolved
Copy link
Contributor

@gogonzo gogonzo left a comment

Choose a reason for hiding this comment

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

Please test cover new functions

NAMESPACE Outdated Show resolved Hide resolved
Copy link
Contributor Author

@averissimo averissimo left a comment

Choose a reason for hiding this comment

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

Some topics to discuss on this implementation

R/qenv-eval_code.R Show resolved Hide resolved
R/qenv-get_var.R Show resolved Hide resolved
R/qenv-get_var.R Show resolved Hide resolved
tests/testthat/test-qenv_constructor.R Show resolved Hide resolved
R/qenv-join.R Outdated Show resolved Hide resolved
averissimo and others added 2 commits November 5, 2024 16:28
Co-authored-by: Dawid Kałędkowski <[email protected]>
Signed-off-by: André Veríssimo <[email protected]>
Copy link
Contributor

@gogonzo gogonzo left a comment

Choose a reason for hiding this comment

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

👍

@averissimo averissimo requested a review from gogonzo November 6, 2024 19:00
R/qenv-get_env.R Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
averissimo and others added 3 commits November 7, 2024 15:36
Co-authored-by: Dawid Kałędkowski <[email protected]>
Signed-off-by: André Veríssimo <[email protected]>
Co-authored-by: Dawid Kałędkowski <[email protected]>
Signed-off-by: André Veríssimo <[email protected]>
R/qenv-get_env.R Outdated Show resolved Hide resolved
@averissimo averissimo merged commit e14dc64 into main Nov 8, 2024
30 checks passed
@averissimo averissimo deleted the 333_deprecate_datanames@main branch November 8, 2024 09:57
@github-actions github-actions bot locked and limited conversation to collaborators Nov 8, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
2 participants