add a function assert_igraph()?

[maelle]

This is a non-critical refactoring suggestion: adding a function that asserts whether something is an igraph object, and errors if not.

To replace lines such as

  if (!is_igraph(graph)) {
    stop("Not a graph object")
  }

and the ones that also first checks whether graph is NULL.

[maelle]

If it's created and used, then it'd be easier to look at other error messages to see whether they need improvements (whereas now most error messages are the message above). 🙂

[szhorvat]

A small comment here is that perhaps using the word "assert" in the function name is not ideal.

Assertions are typically a debugging tool. In most languages they can be disabled to improve performance. An assertion failure indicates a programming error (i.e. a bug) and is not supposed to occur.

Yes, I know this is a bit fuzzy. A programming error by whom? Us? A package developer? An interactive user? It's not as clear cut as saying that no R code is supposed to trigger a C-level assertion from C/igraph (which makes the distinction between structured errors and assertions in the C core).

This doesn't feel like a proper assertion to me in the sense that it's a check for an expected misuse by an interactive user.

This comment concerns purely the naming, and keep in mind that I am not very familiar with R and its conventions!

[krlmlr]

How about check_is_igraph() ?

[szhorvat]

Sure. Also, you're the R expert, so if you think "assert" is actually good in the R context, then go with that.

[szhorvat]

Final comment is that in the C core we were trying to follow a universal style for error messages, where they all end in a fullstop, i.e. Not a graph object.

[ntamas]

I support the idea in general. Let me throw in another suggestion for the naming: ensure_igraph() or ensure_igraph_object(). The ensure_ prefix is a convention that I use in Python for functions that ensure that a particular condition or invariant holds from a given point onwards in a code and throw an exception if it is not possible to satisfy the condition.

But I'll be happy with check_is_igraph() or assert_igraph() as well - I leave it up to @krlmlr to decide, I have no strong preferences either way.

[maelle]

@krlmlr any opinion? 😸

[krlmlr]

I don't have a strong opinion. Perhaps ensure_igraph() ?

Do we need to add other assertions, e.g., if a vertex or edge set is compatible with a graph? If yes, we could think of a family of functions, and name them consistently.

[ntamas]

Do we need to add other assertions, e.g., if a vertex or edge set is compatible with a graph?

I can imagine that there will be other assertion functions like that if we start tidying up the codebase. Let's go with ensure_*().

[github-actions]

This old thread has been automatically locked. If you think you have found something related to this, please open a new issue and link to this old issue if necessary.