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

21 convert data from answercode to answer #26

Merged
merged 27 commits into from
Dec 13, 2023

Conversation

PietrH
Copy link
Member

@PietrH PietrH commented Nov 27, 2023

Todo

  • see if select_inspectors is recoded
    - [ ] Add tests for get_records() especially regarding recoding
  • create issue for optionally returning same column names as in iAsset, but set to default FALSE
  • Add issue for bad failure state on bad username
  • Remove test infrastructure

@PietrH PietrH linked an issue Nov 27, 2023 that may be closed by this pull request
@PietrH PietrH self-assigned this Nov 27, 2023
@PietrH PietrH added the enhancement New feature or request label Nov 27, 2023
@SanderDevisscher
Copy link
Collaborator

@PietrH can I test this ?

@SanderDevisscher
Copy link
Collaborator

PS it works

@PietrH
Copy link
Member Author

PietrH commented Dec 11, 2023

Thanks for having a look at it Sander, I'm not ready with this branch yet. I've added a todo list to this PR.

Sorry it's been dragging on. Do you have any experience writing tests with testthat?

@SanderDevisscher
Copy link
Collaborator

Sorry no experience with testthat, however I'm willing to learn.

@PietrH
Copy link
Member Author

PietrH commented Dec 12, 2023

No worries, we can get together about that at some point. There is some information about it in the R packages book: https://r-pkgs.org/testing-basics.html

@PietrH
Copy link
Member Author

PietrH commented Dec 12, 2023

I'll take care of the unit tests for this PR though, don't worry!

@SanderDevisscher
Copy link
Collaborator

No worries, we can get together about that at some point. There is some information about it in the R packages book: https://r-pkgs.org/testing-basics.html

thanks

@PietrH
Copy link
Member Author

PietrH commented Dec 12, 2023

The failure state when you enter the wrong username isn't very elegant:

get_records(access_token = get_access_token(username = "[email protected]"))

#>Error: access_token is not a string (a length one character vector).

@PietrH
Copy link
Member Author

PietrH commented Dec 12, 2023

Select fields are recoded, currently radio and select fields are treated equally.

@PietrH PietrH linked an issue Dec 12, 2023 that may be closed by this pull request
@PietrH
Copy link
Member Author

PietrH commented Dec 12, 2023

I've decided tests are for a seperate PR, in the context of #7 because it'll require a bit more tooling. I'd like to avoid mocking or vcr if at all possible.

@PietrH
Copy link
Member Author

PietrH commented Dec 12, 2023

The failure state when you enter the wrong username isn't very elegant:

get_records(access_token = get_access_token(username = "[email protected]"))

#>Error: access_token is not a string (a length one character vector).

#27

@PietrH
Copy link
Member Author

PietrH commented Dec 12, 2023

Issue for optionally returning verbatim field names as iAsset: #28

@PietrH
Copy link
Member Author

PietrH commented Dec 12, 2023

PS it works

I'm interpreting this as a positive review, and merging to main. If it's broken, please open an issue.

@PietrH PietrH closed this Dec 12, 2023
@PietrH PietrH reopened this Dec 12, 2023
@PietrH PietrH marked this pull request as ready for review December 12, 2023 15:53
@PietrH PietrH merged commit 818e923 into main Dec 13, 2023
12 checks passed
@PietrH PietrH deleted the 21-convert-data-from-answercode-to-answer branch December 13, 2023 10:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Convert data from answercode to answer get_records(): handle list columns
2 participants