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

Feature/ops function #12

Merged
merged 5 commits into from May 9, 2023
Merged

Feature/ops function #12

merged 5 commits into from May 9, 2023

Conversation

lsilvest
Copy link
Collaborator

@lsilvest lsilvest commented May 9, 2023

It seems at first a bit of an ad-hoc function, but actually it is reasonably useful.

@lsilvest lsilvest requested a review from eddelbuettel May 9, 2023 13:16
@codecov
Copy link

codecov bot commented May 9, 2023

Codecov Report

Patch coverage: 100.00% and no project coverage change.

Comparison is base (7f850c3) 100.00% compared to head (2261bdc) 100.00%.

❗ Current head 2261bdc differs from pull request most recent head 38ff721. Consider uploading reports for the commit 38ff721 to get more accurate results

Additional details and impacted files
@@            Coverage Diff            @@
##            master       #12   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files            2         2           
  Lines          304       378   +74     
=========================================
+ Hits           304       378   +74     
Impacted Files Coverage Δ
R/dtts.R 100.00% <100.00%> (ø)
src/align.cpp 100.00% <100.00%> (ø)

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

README.md Show resolved Hide resolved
R/RcppExports.R Outdated Show resolved Hide resolved
Copy link
Owner

@eddelbuettel eddelbuettel left a comment

Choose a reason for hiding this comment

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

Looks lovely. There is one thing we can do better we may as well make part of the PR (or, if we wanted to be extra strict could make it a new one ... but I think we can do this here but I can have my arm twisted 😉 )

Copy link
Owner

@eddelbuettel eddelbuettel left a comment

Choose a reason for hiding this comment

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

Very nice stuff

@eddelbuettel
Copy link
Owner

eddelbuettel commented May 9, 2023

Oh and if you 'squash merge' we retain the tighter git history.

By virtue of a nice (and common) git alias for ls:

edd@rob:~/git/dtts(master)$ git ls
* 7f850c3 - (HEAD -> master, origin/master, origin/HEAD) fix unitialized memory read (#11) (13 days ago) <Leonardo Silvestri>
*   a5fbcfe - Merge pull request #9 from eddelbuettel/feature/small_refactor (7 weeks ago) <Leonardo Silvestri>
|\  
| * f924615 - small simplification removing extra layer also added implicitly (7 weeks ago) <Dirk Eddelbuettel>
|/  
*   ea49578 - Merge pull request #10 from eddelbuettel/feature/cxx_standard (7 weeks ago) <Leonardo Silvestri>
|\  
| * bec1c33 - Roll minor version [ci skip] (7 weeks ago) <Dirk Eddelbuettel>
| * 2d4f17a - No longer set a compilation standard, remove src/Makevars (7 weeks ago) <Dirk Eddelbuettel>
|/  
* 3e56716 - expand / correct installation (1 year, 2 months ago) <Dirk Eddelbuettel>
* 3e0da21 - add badges, polish one example as suggested by @jangorecki (1 year, 2 months ago) <Dirk Eddelbuettel>
* fc62494 - improved README, added time-series subsetting example (#6) (1 year, 2 months ago) <Leonardo Silvestri>
* 4de7ad7 - micro edits (1 year, 4 months ago) <Dirk Eddelbuettel>
* 0192b24 - Feature/utest overlaps duplicates (#4) (1 year, 5 months ago) <Leonardo Silvestri>
* e1321c1 - Add continuous integration via run.sh from r-ci (#3) (1 year, 5 months ago) <Dirk Eddelbuettel>
* c1f2889 - Feature/utests (#2) (1 year, 5 months ago) <Leonardo Silvestri>
* 39ba721 - more test coverage (1 year, 5 months ago) <lsilvest>
* 953a769 - switch to tinytest (1 year, 6 months ago) <Dirk Eddelbuettel>
* 362c391 - export all 'exported' C++ functions with a dot-name (#1) (1 year, 6 months ago) <Dirk Eddelbuettel>
* 342dc67 - updated old version of DESCRIPTION (1 year, 6 months ago) <lsilvest>
* 4a6a2dd - some more tweaks to README (1 year, 6 months ago) <lsilvest>
* d474c5c - better README; fix for grid.align type (1 year, 6 months ago) <lsilvest>
* ccd67c1 - a bit closer on the documentation (SVG works) (1 year, 6 months ago) <lsilvest>
* a87053f - some stuff on the README; checking to see if the SVG image is displayed properly on GitHub (1 year, 6 months ago) <lsilvest>
* 7fc1c6d - R CMD check with no warnings or notes (1 year, 6 months ago) <lsilvest>
* ac64284 - more documentation; full freedom for 'frequency' (prob not necessary); full freedom for align grid (necessary) (1 year, 6 months ago) <lsilvest>
* c482610 - 'grid.align' and 'frequency' work on 'nanoperiod' (1 year, 6 months ago) <lsilvest>
* 713d1df - corrected most utests except frequency; first overlapping dutation test; need exhaustiveness (1 year, 6 months ago) <lsilvest>
* e72f1bf - added option for open/closed intervals for alignment (1 year, 6 months ago) <lsilvest>
* af267be - untangling the first column  stuff; some work on (1 year, 6 months ago) <lsilvest>
* a5a50f8 - signatures for missing arguments; nanoperiod alignment test start (1 year, 6 months ago) <lsilvest>
* 7a4e9dc - align.idx with (1 year, 6 months ago) <lsilvest>
* a48d3cb - start of align with (1 year, 6 months ago) <lsilvest>
* 5df30ad - passes utests; need to implement  and more utests (1 year, 6 months ago) <lsilvest>
* 85dc396 - compiling and running - no complete unit tests (1 year, 6 months ago) <lsilvest>
* 2fbd9ba - correct time index of alignments (3 years, 3 months ago) <lsilvest>
* 1af4a45 - fix align so it doesn't return NULL when but data.table with 0 rows and correct column dimensions (4 years, 1 month ago) <lsilvest>
* 1d0673d - taking out reference to package nanoival as it is now in nanotime (4 years, 9 months ago) <Leonardo Silvestri>
* f1241be - function documentation (6 years ago) <Leonardo Silvestri>
* 6c29bde - start of the README file (6 years ago) <Leonardo Silvestri>
* 22fcc44 - handle exceptions; utest; frequency (6 years ago) <Leonardo Silvestri>
* 9d6f140 - first working version of align_func (6 years ago) <lsilvest>
* b3d09e4 - align.idx working - no checks, no utests (6 years ago) <lsilvest>
edd@rob:~/git/dtts(master)$ 

@lsilvest lsilvest merged commit 07c95b0 into master May 9, 2023
2 checks passed
@lsilvest lsilvest deleted the feature/ops_function branch May 9, 2023 14:37
@eddelbuettel
Copy link
Owner

(I go back and forth even on repos where it is just me. Sometimes I like all the commits in a PR, sometimes I like the one liners. For large repos (such as $work) squashing is great. )

Really nice PR. Thanks for all the work on this!

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.

None yet

2 participants