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.

Sign up for GitHub

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

Jump to bottom

add necessary + relevant files prior to CRAN submission #21

Merged
merged 39 commits into from
May 14, 2021
Merged

add necessary + relevant files prior to CRAN submission #21

merged 39 commits into from
May 14, 2021

Conversation

rrrlw
Copy link
Member

@rrrlw rrrlw commented Sep 1, 2020 •
edited by corybrunson
Loading

Edit:

  • added pkgdown site
  • add tdaunif to Suggests and incorporate a more exotic manifold sample into the PH vignette
  • visual regression tests
  • drop theme_tda

@rrrlw rrrlw self-assigned this Sep 1, 2020
@rrrlw
Copy link
Member Author

rrrlw commented Sep 1, 2020 •
edited
Loading

Thanks @corybrunson for adding NEWS.md in #15 and for introducing me to the Contributor Covenant (v2 included in this repo) - I'll be sure to switch other repos' contribution guidelines + code of conduct soon. I adapted CONTRIBUTING.md from ggalluvial.

Quick note: I added our emails to CODE_OF_CONDUCT.md and used your gmail from DESCRIPTION. If you'd prefer a different email, please edit.

Currently, just awaiting R CMD check results from win-builder (r-devel). No problems w/ it on Travis (Linux), AppVeyor (Windows), or local install (Mac) w/ r-release.

I will update this PR once win-builder results are back and request a formal review (by @corybrunson) prior to merging.

Edit: added pkgdown site

@corybrunson
Copy link
Member

@rrrlw my email is fine as and where it is!

@rrrlw
Copy link
Member Author

rrrlw commented Oct 27, 2020

Just added the cran-comments.md file to the cran-submit branch - please feel free to add any other local, rhub, etc. test environments.

Adding code coverage and tdaunif are the last two tasks prior to initial CRAN submission, correct?

@rrrlw rrrlw changed the base branch from master to main October 27, 2020 19:43
@corybrunson
Copy link
Member

I think so!

@rrrlw
Copy link
Member Author

rrrlw commented Nov 2, 2020

  • fixed a couple merge conflicts
  • PRNG for author order ended up alphabetical, switched in vignettes (arbitrary method, happy to adjust based on your preference)
  • updated pkgdown website
  • updated README.Rmd & corresponding output (default persistence diagram layout has changed)

corybrunson and others added 7 commits November 7, 2020 07:38
@corybrunson
pkg: add tdaunif to suggests + use projective plane in ph vignette
168e343 
NOTE: ggtda functionality does not depend on tdaunif, which is used to generate
example point clouds. Rather, experimental workflows may use manifold samples
from tdaunif, as illustrated in the vignette and possibly future documentation.
@corybrunson
fix(vignette): clarify homology groups in field of characteristic 2
caf3035
@corybrunson
Merge branch 'cran-submit' into tdaunif
d67b719
@corybrunson
doc(readme): distinguish diagonal line and cutoff line in persistence…
48f0be4 
… diagram

NOTE: The original cutoff line may have been wrongly parameterized and later
reparameterized as the common line y=x used as a visual cue. The visual cue is
now dark gray and the goldenrod cutoff line is corrected.
@rrrlw
refs to bibtex
29f236e
@corybrunson
doc(readme): replace dividing line with point marker in persistence d…
9a6151e 
…iagram
@rrrlw
Merge pull request #32 from rrrlw/cutoff2
4fe80a3 
doc(readme): replace dividing line with point marker in persistence diagram
@corybrunson
Merge pull request #29 from rrrlw/tdaunif
8ac046b 
pkg: add tdaunif to suggests + use projective plane in ph vignette
@corybrunson
feat(annulus2d): remove installed dataset; replace uses with tdaunif …
34057eb 
…samples

BREAKING CHANGE: This commit removes all trace of the data set `annulus2d`,
including its generative code in the 'data-raw' folder, its documentation,
and its uses in examples, tests, and vignettes. In its place, different
samples are generated uniformly from a circle (with noise) using stats::unif
and the (co)sine functions or by calling tdaunif::sample_circle.
Some numerical and visual regression tests are updated.
@corybrunson
pkg(rbuildignore): ignore docs (pkgdown website), code of conduct, co…
8ec92c8 
…ntributing
@corybrunson
Merge branch 'cran-submit' into annulus
888332f
rrrlw and others added 2 commits November 13, 2020 16:19
@rrrlw
minor spelling fix
0ad7e4c
@corybrunson
Merge pull request #33 from rrrlw/annulus
6b95eeb 
feat(annulus2d): remove installed dataset; replace uses with tdaunif samples
@rrrlw
Copy link
Member Author

rrrlw commented Nov 16, 2020

Looks like the checklist is complete - what are the steps remaining to complete prior to merging & submitting to CRAN?

@rrrlw
Copy link
Member Author

rrrlw commented Nov 16, 2020

@corybrunson apologies if we've discussed this and I've forgotten - I'm seeing 23 skipped tests on my local machine. Is this expected behavior?

@corybrunson
Copy link
Member

@rrrlw most of the skipped tests are visual regression tests, preceded in the test files by skip_on_travis() and skip_on_appveyor() because the templates to which the tests match the plots (located in tests/figs) are .gitignored. I don't remember if we or i meant for this to be temporary or permanent! Certainly we can make the change on a sub-branch and see how the services handle it. Do you want to make the attempt?

@rrrlw
Copy link
Member Author

rrrlw commented Nov 16, 2020

Happy to give it a shot

@corybrunson corybrunson mentioned this pull request Nov 16, 2020
Closed
@corybrunson
Copy link
Member

@rrrlw i was just using the documentation to guide an experiment with the README and realized that the documentation for the geoms does not include sections on recognized aesthetics. The documentation for ggplot2::geom_linerange() is a good example. For ggalluvial, i have a roxygen2 template for this section that populates the documentation for each geom, but we'd need at least two here (one for the persistence layers, one for the point cloud layers). I expect that many ggplot2 users are used to looking for aesthetic info in this section, so i'd suggest adding it to the geoms' documentation before submitting to CRAN.

I'd be glad to leave this to you if you want to try it out. Or i could probably get to it this coming week.

@rrrlw
Copy link
Member Author

rrrlw commented Nov 22, 2020

I'll give this a shot over the next few days and request your review on the PR when ready, good catch!

@rrrlw rrrlw mentioned this pull request Nov 22, 2020
Closed
@rrrlw rrrlw merged commit 6851c29 into main May 14, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@corybrunson corybrunson Awaiting requested review from corybrunson

Assignees

@rrrlw rrrlw

Labels
documentation
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@rrrlw @corybrunson