Skip to content

Conversation

@dyrpsf
Copy link
Contributor

@dyrpsf dyrpsf commented Jan 16, 2026

This PR addresses part of #251 by porting the improved comp flattening implementation
from the eikept/jsbml fork and wiring it into the current jsbml-comp module.

Main changes

  • Replaced CompFlatteningConverter with the version from the eikept/jsbml fork,
    which provides a more complete flattening implementation for the comp package
    (including proper handling of ReplacedElement/Deletion chains, conversion factors,
    and nested submodels).
  • Added the helper classes used by the new converter:
    • CompFlatteningIDExpander
    • CompFlatteningIDExpanderTS
  • Adjusted the mergeModels method to merge the “current” (parent) model before
    the previously-flattened submodels. This preserves the original parameter ordering,
    so that flattened output matches the existing testFlattening/testN_flat.xml
    expectations (e.g. for test6).

Testing

From this branch I ran:

mvn -pl :jsbml-comp -Dtest=CompFlattenTest#testSpecificFile test
mvn -pl :jsbml-comp -Dtest=CompFlattenTest test

Results:

  • testSpecificFile and the individual flattening/internalisation tests now pass for the local testFlattening and testGathering data.

  • testAllData now exercises the full testFlattening suite; there is still one mismatch reported via Success Testing Model (this was previously flagged in the test with a TODO comment).

  • testInternaliseExternalModelDefinitions_online still errors due to the remote URL (https://sbmlteam.github.io/jsbml/test/data/comp/simple_online_url.xml) returning a 301 HTML redirect instead of SBML; this looks like a test-data/ hosting issue rather than an algorithm problem.

I’ve kept those two behaviours unchanged but can help adjust the tests if you
prefer (e.g. updating the URL to a raw SBML resource or marking the online test
as integration-only).

@dyrpsf dyrpsf closed this Jan 24, 2026
@dyrpsf dyrpsf deleted the improve-flattening-tests branch January 24, 2026 03:28
@dyrpsf dyrpsf restored the improve-flattening-tests branch January 24, 2026 03:42
@dyrpsf
Copy link
Contributor Author

dyrpsf commented Jan 24, 2026

Hi, this PR was automatically closed because I had to reset and clean up my branches while reverting recent changes.
I’ll reopen a fresh PR once the work is re-applied cleanly.
Thanks!

@dyrpsf
Copy link
Contributor Author

dyrpsf commented Jan 24, 2026

Hi everyone,

Just a quick update: everything is now set and all issues have been resolved. The changes have been reapplied cleanly, and this PR, as well as all other PRs I previously mentioned with problems, are now in good shape. Feel free to review at your convenience.

Thanks for your patience!

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants