Fix name collision in maximum_clique#146
Conversation
|
This ended up being trickier than expected because this package still supports Julia 1.9, but Graphs no longer does, so Again, I'm happy to make revisions when you let me know how you'd like to handle the piracy issue. |
|
I'm sorry for the late response. This package has some common graph algorithms that should be implemented in Graphs.jl as menthoned on #128. But there should be some specific algorithms for molecular mining tasks. In the case of My simple idea is to rename conflicting functions in this package (as you mentioned as (1)). By just adding aliases as like |
Graphs v1.14.0 started exporting `maximum_clique`, which is a function that MolecularGraph also exports. The two names collide, which causes problems for MolecularGraph's test suite. This fixes the problem by renaming MolecularGraph's `maximum_clique` to `maximum_clique_mg`. This also adds a bunch of Aqua tests to check for piracy, etc.
|
Sorry for the delay in getting back to this. I've adopted the |
Graphs v1.14.0 started exporting
maximum_clique, which is a function that MolecularGraph also exports. The two names collide, which causes problems for MolecularGraph's test suite.This "fixes" the problem by merging the two functions, having MolecularGraph add new methods to the Graphs function. Currently, this PR should be viewed as a "trial balloon" because there is a major problem: solving the problem this way is piracy. There are several alternatives, including (1) renaming MolecularGraph's function to something else, (2) scoping calls to
MolecularGraph.maximum_cliquein the test suite, or (3) defining a new function in MolecularGraph that calls Graphs' function and then does some post-processing. Let me know your preferred solution and I can implement it.Because this was introducing piracy, I also took the liberty of adding Aqua tests. Currently this highlights quite a few failures (not just the piracy I introduced), so I marked the test as broken. Fixing these should be the subject of a future PR.