Add **safeframe** integration #150
Open
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Replacing the function and reexporting `labels_df` given that it is a crucial piece of functionality for end users.
The `labels()` is reexported from `datatagr`. Reexport is done because the labels function is valuable to the end user of linelist as well.
Reexporting `datatagr::has_label` to be in the **linelis** NAMESPACE and updating the relevant code/docs. Reexport because the `has_label` function is relevant for the pipelines that may be built by the end user.
Updating to the **datatagr** package for handling the lost action. Also reexporting for ease of use for the user.
This commit removes the `R/tag_variable.R` file, which was a remnant of the old tagging system. This file is no longer needed, as it is superceded by `datatagr::label_variables()`
This commit removes the methods that are now part of the datatagr class. It also removes the tests for these. They operate as methods on the class, so there is no need for additional code changes at this time.
Superceded by `R/labels.R`.
Now we check not based on names of the list, but values. This represents the change to using labels instead of tags.
This commit removes the factored out function with the relevant function from the datatagr package.
These tests are now handled by https://github.com/epiverse-trace/safeframe
These tests are handled in https://github.com/epiverse-trace/safeframe now. They are no longer needed in this package.
These functions help to get and update default values for linelists.
This commit updates tests and adds a helper function to `update_defaults()`. This upgrades make_linelist to use the new safeframe implementation while attempting to make for a relatively user friendly interface in handling the defaults reliably. This does introduce the complexity of using the !!! operator.
This function now checks for valid labels based on equivalent permissiveness as make_linelist
This function now checks for valid labels based
on equivalent permissiveness as make_linelist
This commit adapts the validation procedure to function with labels. Handling the defaults is not trivial, and is now done at the variable level.
chartgerink
changed the title
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR is about integrating the breaking changes from
datatagrsafeframe into linelist.The PR is rather extensive and I ended up unraveling a bunch of code. The handling of default variables, building on labelling implemented in safeframe, is not the most elegant. It ends up with standard functionality like:
I bet there are better ways to manage the default variables; maybe I got stuck in my head after thinking about how to solve this. I remain open to revising the approach. The primary goal of this PR was to understand whether the safeframe API was viable for linelist, or whether it needed further changes (verdict: yes it is viable).
This PR took considerable time and near the end I was feeling overwhelmed and demotivated. As a result, it may not be as rigorous in the places I looked last, like the vignettes or the documentation. The integration of safeframe is complete in scope, so I am marking this PR as ready for review to see how to further refine that integration.
Open questions: