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

Diagonal matrix #55

Merged
merged 6 commits into from
Apr 2, 2018
Merged

Diagonal matrix #55

merged 6 commits into from
Apr 2, 2018

Conversation

cimentadaj
Copy link
Contributor

I fixed as_matrix and set diagonal as optional (also in docs), added tests to check that it inherits the correct diagonal and replaced all of the as_matrix calls within other functions to as_matrix(diagonal = 1) to avoid breaking code. From my side check() is successful. Let me know if this is ok, or do you need other changes.

DESCRIPTION Outdated
@@ -1,6 +1,6 @@
Package: corrr
Type: Package
Version: 0.2.1.9000
Version: 0.3.0.9999
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please change this back to 0.2.1.9000. I tend to work as follows:

  • Start with dev version (ending .9000) and remain on it until a CRAN release
  • For CRAN release change to next version (e.g., 0.3.0)
  • Once on CRAN, flag release on GitHub and change to dev version on next change (by appending .9000)

I tend to follow advice here.

Which reminded me, the next version should actually be 1.0.0 (due to backward compatible issue). Regardless, we'll wait until ready for a CRAN release.

NEWS.md Outdated

# corrr 0.1.0

# corrr 0.3.0.9999
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will also need to change this back

@drsimonj
Copy link
Collaborator

drsimonj commented Apr 1, 2018

Aside, I've also just merged another pull request that may create some conflicts. Please pull origin and check against these.

@cimentadaj
Copy link
Contributor Author

Ok, merged changes and switched versioning to previous version.

@drsimonj drsimonj merged commit d55b275 into tidymodels:master Apr 2, 2018
@drsimonj
Copy link
Collaborator

drsimonj commented Apr 2, 2018

Thanks for fixing and another nice addition to corrr!

drsimonj pushed a commit that referenced this pull request Apr 2, 2018
After two merged PRs (#54 and #55), a few minor edits have been made. A removal of a missed conflict hash in NEWS, and proper ordering of columns and rows after correlate executed with DB backend.
@github-actions
Copy link

github-actions bot commented Mar 6, 2021

This pull request has been automatically locked. If you believe you have found a related problem, please file a new issue (with a reprex: https://reprex.tidyverse.org) and link to this issue.

@github-actions github-actions bot locked and limited conversation to collaborators Mar 6, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants