-
-
Notifications
You must be signed in to change notification settings - Fork 200
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
issues from CRAN checks #1419
Comments
https://www.stats.ox.ac.uk/pub/bdr/Intel/igraph.out More context in https://www.stats.ox.ac.uk/pub/bdr/Intel/igraph.log . Haven't managed to replicate with the Intel 2023.2 compiler from https://github.com/r-hub/containers/tree/main/containers/intel . Same problem in duckdb. |
Here's the warning from the Intel compiler:
As far as I can tell this is not our problem, but an issue with the combination of Intel CC and libstdc++ on that specific system. We use the We normally compile the C core in C++11 mode. To double-check, I compiled in C++17 mode both with GCC 13 (libstdc++) and Clang 18 (libc++), and I see no warning. Feel free to link to this message when submitting to CRAN. I'm happy to take another look if someone can point out a specific issue with the usage of |
I believe the SETLENGTH calls comes from code distributed by |
The PREXPR call comes code included in igraph, which seems to have been borrowed from |
@Antonov548 do you think some stuff should be fixed on igraph's side? |
Yes, the PREXPR issue can only be fixed on igraph's side because it's in C code that's included in igraph (basically a vendored version of https://github.com/hadley/lazyeval). However, as I said above, it may be possible to strip this out and replace it with functionality from |
could you please point me to one of the uses of PREXPR? |
Are you deeply familiar with R's C API (I'm definitely not)? If not, this is not what you should look at. PREXPR is in src/lazyeval.c, which as I said is a vendored version of https://github.com/hadley/lazyeval I suggest you look at what R functions R/lazyeval.R (the R counterpart of src/lazyeval.c) provides. See which of these are used and how they are used. See if these can be replaced with whatever the current In particular, see the use of |
I didn't see immediately your comment about vendoring of package. Good to know it's problem not only in R/igraph. I also will take a look what could be alternatives in R's C API. |
I don't think this is a good use of our time. See the issue I linked to (both in this thread and in the past). If the maintainers of packages like At the risk of sounding like a broken record, I'll say it one last time: I think the only reasonable way forward is to transition to using |
Make sense. I also just not sure what can be used from |
I am not sure when exactly but yes I know a bit of rlang, enough to look into this |
thanks @mpadge! |
I still intend to tackle #1426... but not today. |
https://cran.r-project.org/web/checks/check_results_igraph.html
The text was updated successfully, but these errors were encountered: