krlmlr
commented
11 months ago No, we need to fix those.
@Antonov548: can you please take a look?
Closed maelle closed 10 months ago
krlmlr
commented
11 months ago No, we need to fix those.
@Antonov548: can you please take a look?
maelle
commented
11 months ago Why doesn't it show up in the GitHub Actions workflow @krlmlr? I'm still confused.
Antonov548
commented
11 months ago please ta
Yes, I will take a look
Antonov548
commented
11 months ago @krlmlr Some of the deprecation are required breaking changes, for example changing type of arguments and etc. I guess I will prepare separate pull request for seperate discussion.
szhorvat
commented
10 months ago I hope the following is useful. These are recommendations. The final decision is of course with @krlmlr
rinterface_extra.c:5004:3: warning: ‘igraph_erdos_renyi_game’ is deprecated [-Wdeprecated-declarations]
I suggest auto-generating sample_gnm and sample_gnp from igraph_erdos_renyi_game_gnm() and igraph_erdos_renyi_game_gnp() and implementing the deprecated erdos.renyi.game R function in pure R in terms of sample_gnm and sample_gnp.
This will not be a breaking change on the R side.
rinterface_extra.c:8327:3: warning: ‘igraph_laplacian’ is deprecated [-Wdeprecated-declarations]
Has new normalization options which would be nice to expose. This is definitely a breaking change.
rinterface.c:3853:3: warning: ‘igraph_hub_score’ is deprecated [-Wdeprecated-declarations] rinterface.c:3902:3: warning: ‘igraph_authority_score’ is deprecated [-Wdeprecated-declarations]
These two measures can be computed in tandem (saving time) and should be computed in tandem to ensure that they match in cases when there is more than one valid result. The ideal solution here would set up a new R function that computes them at the same time.
This would not be a breaking change. It would add a new function, but the original hub_score and authority_score don't need to be removed or even deprecated. However, it is good to point out that these two scores are normally interpreted together. So when a user wants one, they also need the other. Unfortunately, many users misunderstand hub_score and try to use it on undirected graphs to find vertices which are "hubs" in a different sense. Deprecating hub_score would avoid this type of confusion.
rinterface.c:5300:3: warning: ‘igraph_random_edge_walk’ is deprecated [-Wdeprecated-declarations]
Again, it might be useful to have a single function which returns both an "edge path" and a "vertex path", similar to the output of shortest_paths. If you go this route, make sure that the naming of fields in the result is consistent with other functions. As I recall, some results used epath/vpath and some used edges/vertices.
If you go with changing random_walk(), that is breaking (although perhaps preferable in the long term).
If you go with keeping random_walk() and random_edge_walk() separate, that is not breaking.
I strongly recommend having a function which returns both the edge and vertex paths. The edge path cannot be derived from the vertex path in case of multigaphs. The vertex path can be derived from the edge path, but it is convenient to do so, because one needs to select the correct endpoint of undirected edges at every step. We have a helper function in C that does this, igraph_expand_path_to_pairs(). You can consider exposing it.
rinterface.c:5927:3: warning: ‘igraph_bipartite_game’ is deprecated [-Wdeprecated-declarations]
See #630. This won't be a breaking change.
The ideal solution would introduce new separate functions, but that's still not breaking if we keep the old one (perhaps deprecate it).
rinterface.c:11853:3: warning: ‘igraph_hrg_dendrogram’ is deprecated [-Wdeprecated-declarations]
The now-deprecated igraph_hrg_dendrogram() created a graph with hard-coded attribute names. The new principle for the C core is to return information as separate vectors, not as attributes with hard-coded names. This ensures that the function can be used without an attribute handler.
In R, you don't have to avoid attributes. You can continue returning the result in the same format.
So the fix to this does not have to be breaking (in fact I recommend keeping the result as it was).
rinterface.c:9970:3: warning: ‘igraph_subisomorphic_function_vf2’ is deprecated [-Wdeprecated-declarations]
This is just a name change. No breaking change needed.
szhorvat
commented
10 months ago vendor/cigraph/src/core/matrix.c:234:13: warning: ‘igraph_vector_e_tol’ is deprecated [-Wdeprecated-declarations]
This warning comes from the C core and cannot be eliminated in any reasonable way. Don't worry about it. It's a limitation of GCC compared to Clang. Clang only issues warnings when a deprecated function is called from a non-deprecated one. GCC issues a warning unconditionally. See https://gcc.gnu.org/bugzilla/show_bug.cgi?id=79078
krlmlr
commented
10 months ago A more recent list:
rinterface_extra.c:3216:34: warning: incompatible pointer types passing 'int *' to parameter of type 'igraph_integer_t *' (aka 'long long *') [-Wincompatible-pointer-types]
/*p=*/ INTEGER(p), /*i=*/ INTEGER(i),
^~~~~~~~~~
vendor/cigraph/include/igraph_sparsemat.h:276:59: note: passing argument to parameter 'p' here
igraph_integer_t *p, igraph_integer_t *i, igraph_real_t *x, igraph_integer_t nz);
^
rinterface_extra.c:3216:53: warning: incompatible pointer types passing 'int *' to parameter of type 'igraph_integer_t *' (aka 'long long *') [-Wincompatible-pointer-types]
/*p=*/ INTEGER(p), /*i=*/ INTEGER(i),
^~~~~~~~~~
vendor/cigraph/include/igraph_sparsemat.h:276:80: note: passing argument to parameter 'i' here
igraph_integer_t *p, igraph_integer_t *i, igraph_real_t *x, igraph_integer_t nz);
^
rinterface_extra.c:4371:18: warning: 'igraph_lattice' is deprecated [-Wdeprecated-declarations]
IGRAPH_R_CHECK(igraph_lattice(&g, &dimvector, nei, directed, mutual, circular));
^
vendor/cigraph/include/igraph_constructors.h:58:15: note: 'igraph_lattice' has been explicitly marked deprecated here
IGRAPH_EXPORT IGRAPH_DEPRECATED igraph_error_t igraph_lattice(igraph_t *graph, const igraph_vector_int_t *dimvector, igraph_integer_t nei,
^
vendor/igraph_export.h:7:43: note: expanded from macro 'IGRAPH_DEPRECATED'
#define IGRAPH_DEPRECATED __attribute__ ((__deprecated__))
^
rinterface_extra.c:4959:3: warning: 'igraph_erdos_renyi_game' is deprecated [-Wdeprecated-declarations]
igraph_erdos_renyi_game(&g, (igraph_erdos_renyi_t) type, n, porm, directed,
^
vendor/cigraph/include/igraph_games.h:222:15: note: 'igraph_erdos_renyi_game' has been explicitly marked deprecated here
IGRAPH_EXPORT IGRAPH_DEPRECATED igraph_error_t igraph_erdos_renyi_game(
^
vendor/igraph_export.h:7:43: note: expanded from macro 'IGRAPH_DEPRECATED'
#define IGRAPH_DEPRECATED __attribute__ ((__deprecated__))
^
rinterface_extra.c:8282:18: warning: 'igraph_laplacian' is deprecated [-Wdeprecated-declarations]
IGRAPH_R_CHECK(igraph_laplacian(&c_graph, (c_sparse ? 0 : &c_res), (c_sparse ? &c_sparseres : 0), c_normalized, (Rf_isNull(weights) ? 0 : &c_weights)));
^
vendor/cigraph/include/igraph_structural.h:164:15: note: 'igraph_laplacian' has been explicitly marked deprecated here
IGRAPH_EXPORT IGRAPH_DEPRECATED igraph_error_t igraph_laplacian(
^
vendor/igraph_export.h:7:43: note: expanded from macro 'IGRAPH_DEPRECATED'
#define IGRAPH_DEPRECATED __attribute__ ((__deprecated__))
^
5 warnings generated.
rinterface.c:3853:18: warning: 'igraph_hub_score' is deprecated [-Wdeprecated-declarations]
IGRAPH_R_CHECK(igraph_hub_score(&c_graph, &c_vector, &c_value, c_scale, (Rf_isNull(weights) ? 0 : &c_weights), &c_options));
^
vendor/cigraph/include/igraph_centrality.h:193:1: note: 'igraph_hub_score' has been explicitly marked deprecated here
IGRAPH_DEPRECATED IGRAPH_EXPORT igraph_error_t igraph_hub_score(const igraph_t *graph, igraph_vector_t *vector,
^
vendor/igraph_export.h:7:43: note: expanded from macro 'IGRAPH_DEPRECATED'
#define IGRAPH_DEPRECATED __attribute__ ((__deprecated__))
^
rinterface.c:3902:18: warning: 'igraph_authority_score' is deprecated [-Wdeprecated-declarations]
IGRAPH_R_CHECK(igraph_authority_score(&c_graph, &c_vector, &c_value, c_scale, (Rf_isNull(weights) ? 0 : &c_weights), &c_options));
^
vendor/cigraph/include/igraph_centrality.h:197:1: note: 'igraph_authority_score' has been explicitly marked deprecated here
IGRAPH_DEPRECATED IGRAPH_EXPORT igraph_error_t igraph_authority_score(const igraph_t *graph, igraph_vector_t *vector,
^
vendor/igraph_export.h:7:43: note: expanded from macro 'IGRAPH_DEPRECATED'
#define IGRAPH_DEPRECATED __attribute__ ((__deprecated__))
^
rinterface.c:5300:18: warning: 'igraph_random_edge_walk' is deprecated [-Wdeprecated-declarations]
IGRAPH_R_CHECK(igraph_random_edge_walk(&c_graph, (Rf_isNull(weights) ? 0 : &c_weights), &c_edgewalk, c_start, c_mode, c_steps, c_stuck));
^
vendor/cigraph/include/igraph_paths.h:354:15: note: 'igraph_random_edge_walk' has been explicitly marked deprecated here
IGRAPH_EXPORT IGRAPH_DEPRECATED igraph_error_t igraph_random_edge_walk(const igraph_t *graph,
^
vendor/igraph_export.h:7:43: note: expanded from macro 'IGRAPH_DEPRECATED'
#define IGRAPH_DEPRECATED __attribute__ ((__deprecated__))
^
rinterface.c:5927:18: warning: 'igraph_bipartite_game' is deprecated [-Wdeprecated-declarations]
IGRAPH_R_CHECK(igraph_bipartite_game(&c_graph, &c_types, c_type, c_n1, c_n2, c_p, c_m, c_directed, c_mode));
^
vendor/cigraph/include/igraph_bipartite.h:102:15: note: 'igraph_bipartite_game' has been explicitly marked deprecated here
IGRAPH_EXPORT IGRAPH_DEPRECATED igraph_error_t igraph_bipartite_game(
^
vendor/igraph_export.h:7:43: note: expanded from macro 'IGRAPH_DEPRECATED'
#define IGRAPH_DEPRECATED __attribute__ ((__deprecated__))
^
rinterface.c:6354:84: warning: variable 'c_outfile' is uninitialized when used here [-Wuninitialized]
IGRAPH_R_CHECK(igraph_maximal_cliques_subset(&c_graph, &c_subset, &c_res, &c_no, c_outfile, c_min_size, c_max_size));
^~~~~~~~~
./rinterface.h:40:35: note: expanded from macro 'IGRAPH_R_CHECK'
igraph_error_type_t __c = func; \
^~~~
rinterface.c:6336:18: note: initialize the variable 'c_outfile' to silence this warning
FILE* c_outfile;
^
= NULL
rinterface.c:8318:58: warning: variable 'c_instream' is uninitialized when used here [-Wuninitialized]
IGRAPH_R_CHECK(igraph_read_graph_dimacs_flow(&c_graph, c_instream, &c_problem, &c_label, &c_source, &c_target, &c_capacity, c_directed));
^~~~~~~~~~
./rinterface.h:40:35: note: expanded from macro 'IGRAPH_R_CHECK'
igraph_error_type_t __c = func; \
^~~~
rinterface.c:8286:19: note: initialize the variable 'c_instream' to silence this warning
FILE* c_instream;
^
= NULL
rinterface.c:8378:59: warning: variable 'c_outstream' is uninitialized when used here [-Wuninitialized]
IGRAPH_R_CHECK(igraph_write_graph_dimacs_flow(&c_graph, c_outstream, c_source, c_target, &c_capacity));
^~~~~~~~~~~
./rinterface.h:40:35: note: expanded from macro 'IGRAPH_R_CHECK'
igraph_error_type_t __c = func; \
^~~~
rinterface.c:8365:20: note: initialize the variable 'c_outstream' to silence this warning
FILE* c_outstream;
^
= NULL
rinterface.c:10810:33: warning: variable 'c_a' is uninitialized when used here [-Wuninitialized]
c_result=igraph_almost_equals(c_a, c_b, c_eps);
^~~
rinterface.c:10802:13: note: initialize the variable 'c_a' to silence this warning
double c_a;
^
= 0.0
rinterface.c:10810:38: warning: variable 'c_b' is uninitialized when used here [-Wuninitialized]
c_result=igraph_almost_equals(c_a, c_b, c_eps);
^~~
rinterface.c:10803:13: note: initialize the variable 'c_b' to silence this warning
double c_b;
^
= 0.0
rinterface.c:10810:43: warning: variable 'c_eps' is uninitialized when used here [-Wuninitialized]
c_result=igraph_almost_equals(c_a, c_b, c_eps);
^~~~~
rinterface.c:10804:15: note: initialize the variable 'c_eps' to silence this warning
double c_eps;
^
= 0.0
rinterface.c:10829:7: warning: variable 'c_result' set but not used [-Wunused-but-set-variable]
int c_result;
^
rinterface.c:10834:31: warning: variable 'c_a' is uninitialized when used here [-Wuninitialized]
c_result=igraph_cmp_epsilon(c_a, c_b, c_eps);
^~~
rinterface.c:10826:13: note: initialize the variable 'c_a' to silence this warning
double c_a;
^
= 0.0
rinterface.c:10834:36: warning: variable 'c_b' is uninitialized when used here [-Wuninitialized]
c_result=igraph_cmp_epsilon(c_a, c_b, c_eps);
^~~
rinterface.c:10827:13: note: initialize the variable 'c_b' to silence this warning
double c_b;
^
= 0.0
rinterface.c:10834:41: warning: variable 'c_eps' is uninitialized when used here [-Wuninitialized]
c_result=igraph_cmp_epsilon(c_a, c_b, c_eps);
^~~~~
rinterface.c:10828:15: note: initialize the variable 'c_eps' to silence this warning
double c_eps;
^
= 0.0
rinterface.c:10841:10: warning: variable 'r_result' is uninitialized when used here [-Wuninitialized]
return(r_result);
^~~~~~~~
rinterface.c:10830:16: note: initialize the variable 'r_result' to silence this warning
SEXP r_result;
^
= NULL
rinterface.c:11611:34: warning: a function declaration without a prototype is deprecated in all versions of C [-Wstrict-prototypes]
SEXP R_igraph_has_attribute_table() {
^
void
rinterface.c:11637:18: warning: unused variable 'c_result' [-Wunused-variable]
igraph_error_t c_result;
^
rinterface.c:11649:10: warning: variable 'r_result' is uninitialized when used here [-Wuninitialized]
return(r_result);
^~~~~~~~
rinterface.c:11638:16: note: initialize the variable 'r_result' to silence this warning
SEXP r_result;
^
= NULL
rinterface.c:11659:18: warning: unused variable 'c_result' [-Wunused-variable]
igraph_error_t c_result;
^
rinterface.c:11671:10: warning: variable 'r_result' is uninitialized when used here [-Wuninitialized]
return(r_result);
^~~~~~~~
rinterface.c:11660:16: note: initialize the variable 'r_result' to silence this warning
SEXP r_result;
^
= NULL
rinterface.c:11680:15: warning: variable 'c_result' set but not used [-Wunused-but-set-variable]
const char* c_result;
^
rinterface.c:11685:28: warning: variable 'c_igraph_errno' is uninitialized when used here [-Wuninitialized]
c_result=igraph_strerror(c_igraph_errno);
^~~~~~~~~~~~~~
rinterface.c:11679:3: note: variable 'c_igraph_errno' is declared here
igraph_error_t c_igraph_errno;
^
rinterface.c:11692:10: warning: variable 'r_result' is uninitialized when used here [-Wuninitialized]
return(r_result);
^~~~~~~~
rinterface.c:11681:16: note: initialize the variable 'r_result' to silence this warning
SEXP r_result;
^
= NULL
rinterface.c:11780:22: warning: a function declaration without a prototype is deprecated in all versions of C [-Wstrict-prototypes]
SEXP R_igraph_version() {
^
void
rinterface.c:11800:31: warning: variable 'version_string' is uninitialized when used here [-Wuninitialized]
SET_VECTOR_ELT(r_result, 0, version_string);
^~~~~~~~~~~~~~
rinterface.c:11786:22: note: initialize the variable 'version_string' to silence this warning
SEXP version_string;
^
= NULL
rinterface.c:11801:31: warning: variable 'major' is uninitialized when used here [-Wuninitialized]
SET_VECTOR_ELT(r_result, 1, major);
^~~~~
rinterface.c:11787:13: note: initialize the variable 'major' to silence this warning
SEXP major;
^
= NULL
rinterface.c:11802:31: warning: variable 'minor' is uninitialized when used here [-Wuninitialized]
SET_VECTOR_ELT(r_result, 2, minor);
^~~~~
rinterface.c:11788:13: note: initialize the variable 'minor' to silence this warning
SEXP minor;
^
= NULL
rinterface.c:11803:31: warning: variable 'subminor' is uninitialized when used here [-Wuninitialized]
SET_VECTOR_ELT(r_result, 3, subminor);
^~~~~~~~
rinterface.c:11789:16: note: initialize the variable 'subminor' to silence this warning
SEXP subminor;
^
= NULL
28 warnings generated.Note that we must eliminate all compiler warnings to keep CRAN happy. The easy route is to temporarily remove the "deprecation" declaration in the vendored C code, and to put it back after release.
krlmlr
commented
10 months ago @Antonov548: Is the code in
rinterface_extra.c:3177:34: warning: passing argument 5 of ‘igraph_sparsemat_view’ from incompatible pointer type [-Wincompatible-pointer-types]used anywhere?
Can we define IGRAPH_DEPRECATED in such a way that will silence the compiler warnings?
Can you please review the other warnings reported by R CMD check ?
https://github.com/igraph/rigraph/actions/runs/7440246978/job/20241096579#step:9:83
@maelle: can you please keep track of the warnings here so that we get down to zero soon-ish?
maelle
commented
10 months ago @Antonov548
From https://github.com/igraph/rigraph/actions/runs/7460708414/job/20299528037?pr=1012 (run on the code from the main branch, since I rebased the PR)
Warning: Found the following significant warnings:
rinterface_extra.c:3177:34: warning: passing argument 5 of ‘igraph_sparsemat_view’ from incompatible pointer type [-Wincompatible-pointer-types]
rinterface_extra.c:3177:53: warning: passing argument 6 of ‘igraph_sparsemat_view’ from incompatible pointer type [-Wincompatible-pointer-types]
See ‘/home/runner/work/rigraph/rigraph/check/igraph.Rcheck/00install.out’ for details. * checking for portable use of $(BLAS_LIBS) and $(LAPACK_LIBS) ... WARNING
##[warning] apparently using $(LAPACK_LIBS) without following $(BLAS_LIBS) in ‘src/Makevars’
apparently using $(BLAS_LIBS) without following $(FLIBS) in ‘src/Makevars’
apparently using $(LAPACK_LIBS) without following $(BLAS_LIBS) in ‘src/Makevars.in’
apparently using $(BLAS_LIBS) without following $(FLIBS) in ‘src/Makevars.in’With #1092, only R warnings/notes remain.
@krlmlr is this expected?