Skip to content

S7 in rvest#436

Open
VisruthSK wants to merge 7 commits into
tidyverse:mainfrom
VisruthSK:main
Open

S7 in rvest#436
VisruthSK wants to merge 7 commits into
tidyverse:mainfrom
VisruthSK:main

Conversation

@VisruthSK

@VisruthSK VisruthSK commented Dec 16, 2024

Copy link
Copy Markdown

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