🛠️ Minimum CXXSTD #89
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
* The Makevars have been updated with a CXX_STD to enable C++23. The original implementation did not enable C++23 features like std::ranges::iota. The current implementation works and adds the appropriate flags during compliation. R CMD check produces no portability warnings.
* The confusion matrix now uses C++23 features and standards as smart pointers, and [[nodiscard]] statements. The unrolled loop iterations have been replaced. The goal is to let the compiler optimize the code. The current implementation has a 5% speed gain over the original implementation.
* This might not solve R CMD check but it will solve local installations via {pak}
* The header-file were constructing the matrix as table(predicted, actual) - the original, in intended method was table(actual, predicted).
* While scouting for errors, the functions have been changed to snake_case. Thats the new thang!
* clang complains about OpenMP - I though everything was handled by gcc in that run; but turns out it was not. Either R has to be built from source (I think), or clang are to be built differently. See: https://stackoverflow.com/questions/60005176/how-to-deal-with-clang-error-unsupported-option-fopenmp-on-travis
* Honestly - this is a wild guess, but logically it should work.
* This fix is purely experimental as it has only been tested on a single machine. It currently works on 15.3.2 (M1) with R version 4.4.3 locally. * Disabled Ubuntu and Windows for faster checks. Remember to readd them. NOTE: This might not fix the significant warnings from Eigen.
* Every aspect of the compilation is currently handled by autconf. The autoconf will recursively check for CXXSTD 23 through 17, if 17 not available it will fail. * OpenMP support tested via a M4 macro, it is not clear what will happen on clang (Not documented, I think)
* The R CMD check now checks everything. * Links to CXX23 have been added. NOTE: Maybe use R CMD config to construct makevars?
* It now includes version and package name. Should be dynamic, but thats for later. I need to celeberate this win.
* Something about line-ending in Windows. See: https://www.aleksandrhovhannisyan.com/blog/crlf-vs-lf-normalizing-line-endings-in-git/ * Updated .gitignore to include src/Makevars; iit is being generated by R CMD build
* c++17 was a 4.0.0 feature. So the minimum version have been changed.
* This is the first attempt to create a configure script for Windows. It is built on the previous Makevars from Main, but uses Shell scripts to determine the CXXSTD. * The cleanup script have been updated accordingly.
* The updated configure.win compiles a program that outputs the C++ version
* All platforms now tests R 4.0.0 as this is the minimum required version (which ships C++17)
* It now includes cpp-file and executable.
* This is just to debug all the sudden of fails. The next step is to change minimum R-version.
* Minimum R-version: 4.0.0 - it should be possible to build and install on this version, but it is not possible to run all the relevant unit-tests directly as the {reticulate} and {pak} dependencies are misbehaving. This is platform and compiler specific it seems. See for example the R CMD checks on 0b6d75f
* SystemRequirements: C++17 - it will **not** build and install if later than that. End of story.
* On Windows and macOS the checks are done according to CRAN: r-devel, r-release and r-oldrel. If you chose to have software that is older than a year - well, then its on you to make it work. * On Linux: r-devel through r-oldrel-3 are tested with default compilers. If it is broken on Linux its most likely broken on MacOS and Windows too. So that will be the baseline.
![ellipsis-dev[bot]](https://avatars.githubusercontent.com/in/64358?s=60&v=4){kind=small}
There was a problem hiding this comment.
❌ Changes requested. Reviewed everything up to dee1aac in 3 minutes and 10 seconds
More details
- Looked at
9218lines of code in22files - Skipped
1files when reviewing. - Skipped posting
22drafted comments based on config settings.
1. src/classification_BalancedAccuracy.h:52
- Draft comment:
Both branches of the ternary operator on L52 are identical. Was this intended? - Reason this comment was not posted:
Comment was not on a location in the diff, so it can't be submitted as a review comment.
2. src/classification_Helpers.h:17
- Draft comment:
Avoid 'using namespace Rcpp' in a header file to prevent potential namespace pollution. - Reason this comment was not posted:
Comment was not on a location in the diff, so it can't be submitted as a review comment.
3. src/classification_ConfusionMatrix.cpp:17
- Draft comment:
Good use of smart pointers with std::make_unique for ConfusionMatrixClass. - Reason this comment was not posted:
Confidence changes required:0%<= threshold50%
None
4. src/classification_ConfusionMatrix.h:102
- Draft comment:
Good use of OpenMP reduction with a custom 'matrixPlus' reduction; ensure that your OpenMP version and target platforms support this. - Reason this comment was not posted:
Confidence changes required:0%<= threshold50%
None
5. src/classification_Helpers.h:17
- Draft comment:
Avoid using 'using namespace Rcpp;' in a header file to prevent namespace pollution. Prefer fully qualifying names or limiting the scope. - Reason this comment was not posted:
Comment was not on a location in the diff, so it can't be submitted as a review comment.
6. src/classification_Helpers.h:190
- Draft comment:
The 'recipe' template uses the micro parameter to call cook.compute(matrix, bool, ...). However, balanced_accuracy only overrides compute(matrix) (without the extra bool parameter). If 'micro' is provided, the call will fall back to the base implementation (returning an empty vector). Consider overriding compute(matrix, bool) in balanced_accuracy or reviewing the design. - Reason this comment was not posted:
Comment was not on a location in the diff, so it can't be submitted as a review comment.
7. src/classification_Helpers.h:71
- Draft comment:
Using the always_inline attribute (via attribute((always_inline))) may reduce portability on non-GCC/Clang compilers. Consider conditional compilation guards for compiler-specific attributes. - Reason this comment was not posted:
Comment was not on a location in the diff, so it can't be submitted as a review comment.
8. .Rbuildignore:26
- Draft comment:
Typographical Error: The pattern '^NEWs_files' on line 26 seems inconsistent with '^NEWS_files' on line 31 (and possibly with your intended naming). Please check if the lowercase 's' is a typo and update it accordingly. - Reason this comment was not posted:
Comment was not on a location in the diff, so it can't be submitted as a review comment.
9. .gitattributes:1
- Draft comment:
Minor: There's a trailing whitespace after 'auto' on line 1. Please remove it to maintain consistency. - Reason this comment was not posted:
Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 10% vs. threshold = 50%
While trailing whitespace is generally undesirable, .gitattributes is a configuration file where trailing whitespace has no functional impact. The comment is technically correct but extremely minor and doesn't affect functionality. This feels like unnecessary nitpicking that doesn't improve code quality.
Trailing whitespace could be considered a cleanliness issue that's worth fixing while we're here. Some might argue that consistency in all files, including configs, is important.
While clean code is important, this level of nitpicking on a non-functional aspect of a configuration file doesn't provide meaningful value and could distract from more important review comments.
The comment should be removed as it points out an extremely minor formatting issue that has no functional impact and doesn't meaningfully improve code quality.
10. .github/workflows/R-CMD-check.yaml:73
- Draft comment:
Typo: 'eveything' should be corrected to 'everything'. - Reason this comment was not posted:
Comment was not on a location in the diff, so it can't be submitted as a review comment.
11. .github/workflows/R-CMD-check.yaml:76
- Draft comment:
Typo: 'creatre' should be corrected to 'create'. - Reason this comment was not posted:
Comment was not on a location in the diff, so it can't be submitted as a review comment.
12. R/RcppExports.R:330
- Draft comment:
Typo detected in function call:_SLmetrics_weigthed_MatthewsCorrelationCoefficient. Consider renaming it to_SLmetrics_weighted_MatthewsCorrelationCoefficient. - Reason this comment was not posted:
Comment was not on a location in the diff, so it can't be submitted as a review comment.
13. R/RcppExports.R:387
- Draft comment:
Typo detected in function call:_SLmetrics_NegativePredictitveValue. Consider correcting it to_SLmetrics_NegativePredictiveValue. - Reason this comment was not posted:
Comment was not on a location in the diff, so it can't be submitted as a review comment.
14. R/RcppExports.R:393
- Draft comment:
Typo detected in function call:_SLmetrics_weighted_NegativePredictitveValue. Consider renaming it to_SLmetrics_weighted_NegativePredictiveValue. - Reason this comment was not posted:
Comment was not on a location in the diff, so it can't be submitted as a review comment.
15. cleanup.win:7
- Draft comment:
The file is missing a trailing newline at the end. While this is a minor cosmetic issue, it's generally good practice to end text files with a newline to maintain consistency across tools and environments. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =0%<= threshold50%
This comment is purely informative and does not provide a specific actionable suggestion related to the code's functionality or correctness. It does not align with the rules provided, which discourage purely informative comments.
16. configure:1
- Draft comment:
Below is the complete new file content for 'configure' after the diff is applied. It is very long (over 6900 lines) and contains all changes as generated by Autoconf. Please review the entire file carefully to ensure it meets your expectations. If you need only a summary or a diff of critical parts, let me know. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =0%<= threshold50%
This comment is purely informative and asks the PR author to review the file carefully, which violates the rules. It doesn't provide any specific code suggestions or improvements.
17. configure.win:40
- Draft comment:
Typographical error: The comment contains a duplicated word "a a good idea". Please remove the extra 'a' to improve clarity. - Reason this comment was not posted:
Marked as duplicate.
18. src/RcppExports.cpp:635
- Draft comment:
Typographical error: Rename 'weigthed_MatthewsCorrelationCoefficient' to 'weighted_MatthewsCorrelationCoefficient' to correct the spelling. - Reason this comment was not posted:
Comment was not on a location in the diff, so it can't be submitted as a review comment.
19. src/RcppExports.cpp:730
- Draft comment:
Typographical error: Rename 'NegativePredictitveValue' to 'NegativePredictiveValue' to fix the spelling mistake. - Reason this comment was not posted:
Comment was not on a location in the diff, so it can't be submitted as a review comment.
20. src/classification_Helpers.h:26
- Draft comment:
Typo: 'imporant' should be corrected to 'important'. - Reason this comment was not posted:
Comment was not on a location in the diff, so it can't be submitted as a review comment.
21. src/classification_Helpers.h:33
- Draft comment:
Typo: 'wether' should be corrected to 'whether'. - Reason this comment was not posted:
Comment was not on a location in the diff, so it can't be submitted as a review comment.
22. src/classification_Helpers.h:34
- Draft comment:
Typo: 'rerrturned' should be corrected to 'returned'. - Reason this comment was not posted:
Comment was not on a location in the diff, so it can't be submitted as a review comment.
Workflow ID: wflow_ykiOZQ1GELf2Nfsd
Want Ellipsis to fix these issues? Tag @ellipsis-dev in a comment. You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.
There was a problem hiding this comment.
❌ Changes requested. Incremental review on 7374678 in 1 minute and 56 seconds
More details
- Looked at
198lines of code in6files - Skipped
0files when reviewing. - Skipped posting
10drafted comments based on config settings.
1. src/classification_BalancedAccuracy.cpp:14
- Draft comment:
Passing booleans by const reference is unnecessary; consider passing 'adjust' by value for consistency and efficiency. (Evidence: parameter declaration 'const bool& adjust') - Reason this comment was not posted:
Comment was on unchanged code.
2. src/classification_BalancedAccuracy.h:38
- Draft comment:
The expression 'temp.isNaN() == false' is unclear; consider using a more direct method like '(!temp.isNaN()).count()' to count non-NaN entries. - Reason this comment was not posted:
Comment was not on a location in the diff, so it can't be submitted as a review comment.
3. src/classification_BalancedAccuracy.h:52
- Draft comment:
The ternary operator for 'output' returns the same expression for both branches. Verify if a different behavior is intended when na_rm is false, or simplify the code. - Reason this comment was not posted:
Comment was not on a location in the diff, so it can't be submitted as a review comment.
4. src/classification_BalancedAccuracy.h:7
- Draft comment:
It is recommended to define EIGEN_USE_MKL_ALL before including <RcppEigen.h> so that the MKL settings are applied correctly. Consider moving the #define line above the Eigen (or RcppEigen) include. - Reason this comment was not posted:
Comment was on unchanged code.
5. src/classification_BalancedAccuracy.h:10
- Draft comment:
Names beginning with an underscore can be reserved. Consider renaming balanced_accuracy to something like balanced_accuracy_impl to avoid potential conflicts. - Reason this comment was not posted:
Marked as duplicate.
6. src/classification_BalancedAccuracy.h:52
- Draft comment:
Both branches of the ternary operator computing the output are identical. Remove the redundant condition and simply assign the constant value. - Reason this comment was not posted:
Comment was not on a location in the diff, so it can't be submitted as a review comment.
7. configure.win:26
- Draft comment:
For improved robustness and readability, use modern command substitution syntax (i.e. $(...)) and quote variables when capturing command output. - Reason this comment was not posted:
Comment was not on a location in the diff, so it can't be submitted as a review comment.
8. configure.ac:72
- Draft comment:
Typographical error: In the OpenMP support comment (line 72), 'this have not been' should be corrected to 'this has not been'. - Reason this comment was not posted:
Comment was on unchanged code.
9. configure.ac:88
- Draft comment:
Typographical error: In the zlib check comment (line 88), 'its' should be corrected to 'it's' to use the proper contraction for 'it is'. - Reason this comment was not posted:
Comment was on unchanged code.
10. configure.win:40
- Draft comment:
Typographical error: The note on line 40 contains a duplicated 'a' ('a a good idea'). Please remove one of the 'a's for clarity. - Reason this comment was not posted:
Comment was not on a location in the diff, so it can't be submitted as a review comment.
Workflow ID: wflow_T1aDLRqSdXfMVfYK
Want Ellipsis to fix these issues? Tag @ellipsis-dev in a comment. You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.
There was a problem hiding this comment.
👍 Looks good to me! Incremental review on 47bfbaa in 57 seconds
More details
- Looked at
162lines of code in3files - Skipped
0files when reviewing. - Skipped posting
9drafted comments based on config settings.
1. src/classification_BalancedAccuracy.h:51
- Draft comment:
The ternary operator is redundant (both branches are identical). Consider removing the conditional or adjusting one branch if different behavior is intended. - Reason this comment was not posted:
Comment was on unchanged code.
2. src/classification_BalancedAccuracy.h:37
- Draft comment:
Consider adding a safeguard against division by zero when computing 'n_classes'. - Reason this comment was not posted:
Comment was on unchanged code.
3. src/classification_BalancedAccuracy.cpp:15
- Draft comment:
For small types like bool, passing by value is preferred instead of by const reference. - Reason this comment was not posted:
Confidence changes required:50%<= threshold50%
None
4. src/classification_BalancedAccuracy.h:7
- Draft comment:
Consider guarding or conditionally defining '#define EIGEN_USE_MKL_ALL' since MKL may not be available on all platforms. - Reason this comment was not posted:
Comment was on unchanged code.
5. src/classification_BalancedAccuracy.h:51
- Draft comment:
Both branches of the ternary operator are identical; consider simplifying to a single assignment. - Reason this comment was not posted:
Comment was on unchanged code.
6. src/classification_BalancedAccuracy.h:37
- Draft comment:
Ensure that 'n_classes' is non-zero before dividing to avoid potential division by zero errors. - Reason this comment was not posted:
Comment was on unchanged code.
7. src/classification_BalancedAccuracy.cpp:17
- Draft comment:
The use of the helper functions 'TP' and 'recipe' should be verified for correctness; ensure they handle edge cases (e.g., zero row sums) appropriately. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =0%<= threshold50%
This comment is asking the PR author to verify the correctness of helper functions and ensure they handle edge cases, which violates the rule against asking the author to confirm or ensure behavior. It does not provide a specific suggestion or point out a specific issue.
8. src/RcppExports.cpp:635
- Draft comment:
Typographical error: The function name 'weigthed_MatthewsCorrelationCoefficient' is misspelled. It should be 'weighted_MatthewsCorrelationCoefficient' to maintain consistency with typical naming conventions. - Reason this comment was not posted:
Comment was not on a location in the diff, so it can't be submitted as a review comment.
9. src/RcppExports.cpp:730
- Draft comment:
Typographical error: The function name 'NegativePredictitveValue' (and its corresponding weighted version) is misspelled. The correct spelling should be 'NegativePredictiveValue'. Please update both occurrences to fix this typo. - Reason this comment was not posted:
Comment was not on a location in the diff, so it can't be submitted as a review comment.
Workflow ID: wflow_He2UUWvJLAb74OrQ
You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.
* This is closely related to 63ede97 but a more aggresive approach. The Makevars are purely built from Autoconf - so it should not be necessary to build a Makevars i GHA.
There was a problem hiding this comment.
👍 Looks good to me! Incremental review on 73b756b in 48 seconds
More details
- Looked at
53lines of code in1files - Skipped
0files when reviewing. - Skipped posting
4drafted comments based on config settings.
1. .github/workflows/R-CMD-check.yaml:78
- Draft comment:
- Removal of 'Configure Makevars (macOS-gcc)' block: Ensure autoconf now manages these configurations for macOS gcc builds. Verify that gcc-14 settings and OpenMP flags are correctly set in the new setup.
- Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =0%<= threshold50%
The comment is asking the PR author to ensure and verify configurations, which violates the rule against asking for confirmation or verification. It does not provide a specific code suggestion or point out a specific issue that needs addressing.
2. .github/workflows/R-CMD-check.yaml:80
- Draft comment:
- The removal of the 'Configure Makevars (macOS-gcc)' step appears appropriate now that Autoconf handles compiler configuration. Verify that gcc on macOS still receives the correct flags and that no necessary customization is lost.
- Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =0%<= threshold50%
This comment is asking the PR author to verify that gcc on macOS still receives the correct flags and that no necessary customization is lost. This falls under the rule of not asking the PR author to double-check things or ensure behavior is intended. Therefore, this comment should be removed.
3. .github/workflows/R-CMD-check.yaml:73
- Draft comment:
Typographical error: 'eveything' should be corrected to 'everything'. - Reason this comment was not posted:
Comment was not on a location in the diff, so it can't be submitted as a review comment.
4. .github/workflows/R-CMD-check.yaml:76
- Draft comment:
Typographical error: 'creatre' should be corrected to 'create'. - Reason this comment was not posted:
Comment was not on a location in the diff, so it can't be submitted as a review comment.
Workflow ID: wflow_sNaQ8aqnipEFFKf0
You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.
* This step **will** fail on MacOS, as the compiler is not linked to OpenMP in R by default (I think) - this is something that happens during build, and therefore it will fail. However, this is a wild guess as it is my understanding that plugins will link it during build of single programs.
There was a problem hiding this comment.
👍 Looks good to me! Incremental review on 96e35c5 in 1 minute and 13 seconds
More details
- Looked at
35lines of code in1files - Skipped
0files when reviewing. - Skipped posting
4drafted comments based on config settings.
1. .github/workflows/R-CMD-check.yaml:89
- Draft comment:
The removal of the explicit OpenMP availability check looks intentional, as Autoconf now handles C++ standard selection and OpenMP flags. Ensure that documentation reflects this change. - Reason this comment was not posted:
Confidence changes required:0%<= threshold50%
None
2. .github/workflows/R-CMD-check.yaml:96
- Draft comment:
Removed the OpenMP availability check step. Ensure that the autoconf configuration now fully covers the OpenMP flag setup for non-clang compilers, so no runtime issues arise on systems lacking proper OpenMP support. - Reason this comment was not posted:
Confidence changes required:33%<= threshold50%
None
3. .github/workflows/R-CMD-check.yaml:73
- Draft comment:
Typo: In the comment on line 73, change 'eveything' to 'everything'. - Reason this comment was not posted:
Comment was not on a location in the diff, so it can't be submitted as a review comment.
4. .github/workflows/R-CMD-check.yaml:76
- Draft comment:
Typo: In the comment on line 76, change 'creatre' to 'create'. - Reason this comment was not posted:
Comment was not on a location in the diff, so it can't be submitted as a review comment.
Workflow ID: wflow_iKS7dgj2vIyKXuqb
You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.
* See issue #90 - from the build logs r-oldrel-1 uses CXXSTD23, but CXX23 is never really linked during the creation of the local Makevars. This **should** fix the issue altogether.
* The .yaml file has been reformatted so it uses tabs and editor formatting. * Used cat <<- EOF > to fix: /Users/runner/.R/Makevars:28: *** missing separator. Stop.
There was a problem hiding this comment.
👍 Looks good to me! Incremental review on 5b478eb in 1 minute and 2 seconds
More details
- Looked at
316lines of code in1files - Skipped
0files when reviewing. - Skipped posting
6drafted comments based on config settings.
1. .github/workflows/R-CMD-check.yaml:73
- Draft comment:
Typo: "eveything" should be corrected to "everything". - Reason this comment was not posted:
Comment was on unchanged code.
2. .github/workflows/R-CMD-check.yaml:76
- Draft comment:
Typo: "creatre folders" should be corrected to "create folders". - Reason this comment was not posted:
Comment was on unchanged code.
3. .github/workflows/R-CMD-check.yaml:73
- Draft comment:
Typo: 'eveything' should be 'everything'. - Reason this comment was not posted:
Comment was on unchanged code.
4. .github/workflows/R-CMD-check.yaml:76
- Draft comment:
Typo: 'creatre folders' should be 'create folders'. - Reason this comment was not posted:
Comment was on unchanged code.
5. .github/workflows/R-CMD-check.yaml:73
- Draft comment:
Typo: 'eveything' should be corrected to 'everything'. - Reason this comment was not posted:
Comment was on unchanged code.
6. .github/workflows/R-CMD-check.yaml:76
- Draft comment:
Typo: 'creatre' should be corrected to 'create'. - Reason this comment was not posted:
Comment was on unchanged code.
Workflow ID: wflow_IjSuykc5ikRLroT8
You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.
* Adding CXX23 links did not solve the issue. And now another error occured: /Users/runner/.R/Makevars:28: *** missing separator. Stop. This is something about indendation: https://stackoverflow.com/questions/78408580/makevars1-missing-separator-stop-compiling-for-an-r-package-that-is-w 2025, and indendation determines the outcome of a program - not gonna entertain this any longer.
There was a problem hiding this comment.
👍 Looks good to me! Incremental review on 6a0bf5a in 41 seconds
More details
- Looked at
96lines of code in1files - Skipped
0files when reviewing. - Skipped posting
5drafted comments based on config settings.
1. .github/workflows/R-CMD-check.yaml:66
- Draft comment:
Removal of the macOS gcc Makevars configuration block. Confirm that the autoconf setup now fully configures the compiler flags (e.g. OMP, gettext, SDK paths) for macOS with gcc. Ensure no required settings are omitted. - Reason this comment was not posted:
Comment was not on a location in the diff, so it can't be submitted as a review comment.
2. .github/workflows/R-CMD-check.yaml:94
- Draft comment:
Removal of the explicit OpenMP availability check. Ensure that autoconf or another mechanism now reliably detects OpenMP support, and that appropriate fallbacks are in place for systems without OpenMP. - Reason this comment was not posted:
Comment was not on a location in the diff, so it can't be submitted as a review comment.
3. .github/workflows/R-CMD-check.yaml:92
- Draft comment:
Remove the 'needs: check' parameter. 'needs' is a job-level dependency and is not valid in a step. - Reason this comment was not posted:
Comment was not on a location in the diff, so it can't be submitted as a review comment.
4. .github/workflows/R-CMD-check.yaml:73
- Draft comment:
Typo: 'eveything' should be corrected to 'everything'. - Reason this comment was not posted:
Comment was not on a location in the diff, so it can't be submitted as a review comment.
5. .github/workflows/R-CMD-check.yaml:76
- Draft comment:
Typo: 'creatre' should be corrected to 'create'. - Reason this comment was not posted:
Comment was not on a location in the diff, so it can't be submitted as a review comment.
Workflow ID: wflow_IcSPXPjafs38wGcV
You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
📚 What?
This PR sets the minimum
C++-standard usingAutoconftoCXXSTD17and the minimumR-version to4.0.0.Autoconfwill build {SLmetrics} on UNIX systems in the following order:CXXSTD23is available, otherwise fallback toCXXSTD20orCXXSTD17sequentially. The build process will fail if, at minimum,CXXSTD17is not available.Note
The ~/.R/Makevars creation is now only relevant for gcc on
macOSas Autoconf handles everything else.📖 Other changes
The
cmatrix()are now passing pointers to theclassification_ConfusionMatrix.hand is now 5% faster than the original implementation.🗒️ TODO
CXXSTD23which isn't optimal.SystemRequirements-field inDESCRIPTIONMove1M4-macros toinst/or.meta/⚒️ Note
The last working checks in the same setup was commit df8a2d9 - when comparing, the only difference is the addition of
configure.win- in all gcc runs theCXXSTDis set to23Which gives the warnings at R CMD check:
Footnotes
That is for another PR, and another day. ↩