Skip to content

[WIP] Restore compatibility with harmony 2.X #30

Draft
Bisaloo wants to merge 4 commits into
const-ae:develfrom
Bisaloo:clean-harmony-fix
Draft

[WIP] Restore compatibility with harmony 2.X #30
Bisaloo wants to merge 4 commits into
const-ae:develfrom
Bisaloo:clean-harmony-fix

Conversation

@Bisaloo

@Bisaloo Bisaloo commented Jun 16, 2026

Copy link
Copy Markdown
Contributor

This is still WIP but opening here for visibility and so we can easily link to it while we open issues or feature requests in other projects.

The current changes are the results from a collaboration between @Herbermann and myself.

It's a bit fragile because we resorted to copying the harmony class definition. This is currently the only way lemur code can know what the class looks like and where to access/set specific elements, such as Z_corr. However, this means that even a tiny update to the class definition on harmony side will break this code.

Long-term, it would be easier for lemur if harmony:

  • either exposed the class definition in inst/include/, so lemur can LinkingTo harmony and delete the src/harmony_defines.h file created in this PR.
  • or exported a setZcorr() method, the same way it is exporting the getZcorr() method. We can then remove src/harmony_defines.h and src/harmony_bridge.cpp created in this PR.

@const-ae

Copy link
Copy Markdown
Owner

Hey, thanks for the draft. It actually looks better than I feared this would be. Of course it would be good to have something more stable, but this could be a stopgap solution. Do you actually have an overview when the next Bioconductor deadline is coming up so that the package won't be removed?

I have also been wondering if it might be better to just re-implement the maximum discrepancy clustering. Has any of you looked into this already?

@wolfganghuber

wolfganghuber commented Jun 17, 2026 via email

Copy link
Copy Markdown
Collaborator

@Herbermann

Copy link
Copy Markdown

On 16. Jun 2026, at 23:37, Constantin @.***> wrote: const-ae left a comment (const-ae/lemur#30) <#30 (comment)> Hey, thanks for the draft. It actually looks better than I feared this would be. Of course it would be good to have something more stable, but this could be a stopgap solution. Do you actually have an overview when the next Bioconductor deadline is coming up so that the package won't be removed? I have also been wondering if it might be better to just re-implement the maximum discrepancy clustering. Has any of you looked into this already?
Good point. It could be a useful separate package… And should not be too onerous given modern coding tools. I feel ready to support it as a PI till the end of my life cycle, if one of you writes & submits a good implementation. Wolfgang

I played around a little but have not followed up much more at the time, because I feared it will take a bit too long. The algorithm itself is well documented in their paper, including some implementation tricks they used. Creating a package that simply does the diversity clustering step shouldn't be too big in scope. I'll look into it!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants