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

Remove green_or_brown in favour of increasing_or_decreasing #438

Merged
merged 11 commits into from
Jul 19, 2023

Conversation

jdhoffa
Copy link
Member

@jdhoffa jdhoffa commented Jul 5, 2023

Closes #434

@jdhoffa
Copy link
Member Author

jdhoffa commented Jul 5, 2023

It is expected that some of these checks will fail until the latest version of https://github.com/RMI-PACTA/r2dii.data is on CRAN, since we are now depending on that latest dataset. As such, I will convert this to draft until that is the case.

Also, note to self, this package will now depend on a latest version of r2dii.data

@jdhoffa jdhoffa marked this pull request as draft July 5, 2023 07:44
@cjyetman
Copy link
Member

cjyetman commented Jul 5, 2023

Note in README please 😊

@jdhoffa
Copy link
Member Author

jdhoffa commented Jul 5, 2023

Wait a sec, I'm a dingus.

The argument green_or_brown was only added in the development version of this package in the first place and never released, so don't need to deprecate it. Can just change it entirely to increasing_or_decreasing.

I'm a little rusty on working on CRAN-released packages :-D

@jdhoffa jdhoffa changed the title Begin deprecation of green_or_brown in favour of increasing_or_decreasing Remove green_or_brown in favour of increasing_or_decreasing Jul 5, 2023
@jdhoffa jdhoffa marked this pull request as ready for review July 19, 2023 11:40
@jdhoffa
Copy link
Member Author

jdhoffa commented Jul 19, 2023

Ok @cjyetman this is now ready for review.

It was much easier than I anticipated, because green_or_brown was never actually an exported argument of any function in any of our CRAN releases, so I don't actually need to deprecate (I can just remove/ rename it as I wish)

cjyetman
cjyetman previously approved these changes Jul 19, 2023
@cjyetman
Copy link
Member

weird that so many checks failed?

@jdhoffa
Copy link
Member Author

jdhoffa commented Jul 19, 2023

weird that so many checks failed?

This is because of all of the warnings due to the many-to-many relationship, which I handled in a separate PR.

@jdhoffa
Copy link
Member Author

jdhoffa commented Jul 19, 2023

weird that so many checks failed?

This is because of all of the warnings due to the many-to-many relationship, which I handled in a separate PR.

I have now merged that PR in. All checks should pass!

@jdhoffa jdhoffa requested a review from cjyetman July 19, 2023 12:13
@cjyetman
Copy link
Member

looks like all "R-CMD-check r2dii-devel" tests pass, but all "R-CMD-check" tests fail. not sure what the difference is between the two

@jdhoffa
Copy link
Member Author

jdhoffa commented Jul 19, 2023

looks like all "R-CMD-check r2dii-devel" tests pass, but all "R-CMD-check" tests fail. not sure what the difference is between the two

devel uses the development version of dependency packages (e.g. r2dii.data and r2dii.match) to run checks, whereas the normal one uses the CRAN release.

This makes sense, since the functions now require the new dataset increasing_or_decreasing, which only exists in the development version of r2dii.data (it is not yet on CRAN). Once we release r2dii.data to CRAN, all checks should pass.

@jdhoffa jdhoffa merged commit 77b9669 into main Jul 19, 2023
10 of 23 checks passed
@jdhoffa jdhoffa deleted the 434-deprecate_green_or_brown branch July 19, 2023 15:00
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.

Begin deprecation of usage of green_or_brown
2 participants