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

S7 in rvest #436

Open
wants to merge 7 commits into
base: main
Choose a base branch
from
Open

S7 in rvest #436

wants to merge 7 commits into from

Conversation

VisruthSK
Copy link

@VisruthSK VisruthSK commented Dec 16, 2024

Closes #414.

Converted some S3 classes to S7 in form.R and session.R.

Notable changes:

  • The S3 implementation of rvest_field allowed for dots to be passed. I noted that the dots were only ever used to pass one (optional) argument, options; as such, I added options as a new property to the S7 implementation but did not include a way to pass the dots when creating a rvest_field. That is an easy fix if needed.
  • Adjusted tests to match S7 syntax and the like.
  • Bumped Roxygen to 7.3.2.

Issues

  • The "basic session process works as expected" test fails locally. I get the same (incorrect) test output with the original S3 implementation cloned from Github though. I get Size: 821905 instead of Size: 20707 as the snapshot asserts. I think this is an error with the test?
  • No documentation for any S7 pecularities.

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

Successfully merging this pull request may close these issues.

Try S7
1 participant