search

AlgebraicJulia / SyntacticModels.jl

Specifying models with syntax trees
MIT License
3 stars 3 forks source link

WIP update to use InterTypes #29

Closed p-stokes closed 8 months ago

p-stokes commented 10 months ago

This is to make the InterType form of the various structures currently in SyntacticModels.

Resolves #28

codecov[bot] commented 10 months ago

Codecov Report

Attention: 16 lines in your changes are missing coverage. Please review.

Comparison is base (0ea6f64) 81.74% compared to head (e07fb29) 87.11%.

Files Patch % Lines
src/amr.jl 86.44% 8 Missing :warning:
src/decapodes.jl 96.73% 5 Missing :warning:
src/composite_models.jl 87.50% 2 Missing :warning:
src/uwd.jl 50.00% 1 Missing :warning:
Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #29 +/- ## ========================================== + Coverage 81.74% 87.11% +5.37% ========================================== Files 6 6 Lines 378 489 +111 ========================================== + Hits 309 426 +117 + Misses 69 63 -6 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

p-stokes commented 10 months ago

The unnecessary rename of these file makes the diffs unreviewable. Please put them back and confirm that the diffs are useful. I can review again at that time.

I've updated the file names for the PR, and the diffs look ok now. Regarding the included decapodes functions, I think the longer-term plan is for Decapodes and other structures to be defined directly upstream with InterTypes. But for the short-term, it seemed easier to develop locally within SyntacticModels.

I do think it would be helpful to have a discussion about the design/scope plan for SyntacticModels because it relates to the current points of development for the AMR and Composites functionality as well.

p-stokes commented 9 months ago

I cleaned up and standardized the imports and name qualifications. Still working on the remaining composite oapply example and amr acsetspec examples

jpfairbanks commented 9 months ago

I do think it would be helpful to have a discussion about the design/scope plan for SyntacticModels because it relates to the current points of development for the AMR and Composites functionality as well.

I think the first part of that discussion is understanding your expectations for the scope. What are your thoughts?

p-stokes commented 9 months ago

I do think it would be helpful to have a discussion about the design/scope plan for SyntacticModels because it relates to the current points of development for the AMR and Composites functionality as well.

I think the first part of that discussion is understanding your expectations for the scope. What are your thoughts?

Summarizing our in-person discussion about this: The short-term scope is to convert the functionality and test examples to use InterTypes locally within SyntacticModels.jl (as best makes sense and is possible). Longer-term, the .it files and functions will be upstreamed to their respective repositories. There are several blocking issues with InterTypes' capabilities and integration with some other upstream functionality (e.g., ADTs and Open). I will make issues for those and tag them below.

olynch commented 8 months ago

Some of the work here ends up duplicating work merged into DiagrammaticEquations, so it makes sense to make a new PR