Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

refactor(createpackages): use jinja for mf6 module code generation #2333

Draft
wants to merge 79 commits into
base: develop
Choose a base branch
from

Conversation

wpbonelli
Copy link
Member

@wpbonelli wpbonelli commented Oct 10, 2024

Carved out of #2317 — just move to Jinja as an initial step, type hints can come afterwards as they will take some effort to get right. The aim here is to match the old createpackages.py capabilities without relying on mfstructure.py. This is an initial step to move mfstructure.py to a leaner representation of the input spec. mfstructure.py is still used at runtime which will need to be unraveled in followup work.

Introduce a flopy.mf6.utils.codegen module with some new machinery for loading an input specification from DFN files into an intermediate representation, and generating code with Jinja. This includes a "shim" of filters handling quirks of the generated classes which we can aim to gradually eliminate. The templates should also get simpler over time.

The codegen module uses devtools to parse DFNs and convert them to TOML. I figure we can tweak the new format under the hood in flopy until we're happy with it then continue the migration upstream in mf6.

Jinja2 and modflow-devtools are added to a new optional dependency group codegen. These are required to use the code generation utilities.

Miscellaneous:

@wpbonelli wpbonelli force-pushed the jinja branch 2 times, most recently from a3b7c2e to af1e7a5 Compare October 10, 2024 17:49
@wpbonelli wpbonelli added modflow 6 dependencies Pull requests that update a dependency file maintenance labels Oct 10, 2024
@wpbonelli wpbonelli force-pushed the jinja branch 4 times, most recently from d1e8a21 to a7f2644 Compare October 10, 2024 18:30
@wpbonelli wpbonelli added this to the 3.9 milestone Oct 10, 2024
Copy link
Contributor

@deltamarnix deltamarnix left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I haven't looked through everything, but I left you a PR on your own branch with indications on how I would probably approach this. Let's take a look at it together and see if we can get rid of the shim :)

autotest/test_codegen.py Outdated Show resolved Hide resolved
docs/mf6_dev_guide.md Outdated Show resolved Hide resolved
docs/mf6_dev_guide.md Outdated Show resolved Hide resolved
pyproject.toml Outdated Show resolved Hide resolved
flopy/mf6/data/dfn/utl-tas.dfn Outdated Show resolved Hide resolved
flopy/mf6/utils/codegen/templates/context.py.jinja Outdated Show resolved Hide resolved
flopy/mf6/utils/codegen/shim.py Outdated Show resolved Hide resolved
@wpbonelli wpbonelli force-pushed the jinja branch 2 times, most recently from bab83ac to cf68432 Compare October 18, 2024 00:32
Copy link

codecov bot commented Oct 18, 2024

Codecov Report

Attention: Patch coverage is 90.74675% with 57 lines in your changes missing coverage. Please review.

Project coverage is 76.5%. Comparing base (bb9824e) to head (9a25689).
Report is 15 commits behind head on develop.

Files with missing lines Patch % Lines
flopy/mf6/utils/codegen/dfn2toml.py 0.0% 30 Missing ⚠️
flopy/mf6/utils/codegen/filters.py 93.5% 15 Missing ⚠️
flopy/mf6/utils/codegen/dfn.py 96.0% 10 Missing ⚠️
flopy/mf6/utils/createpackages.py 75.0% 1 Missing ⚠️
flopy/mf6/utils/generate_classes.py 83.3% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff            @@
##           develop   #2333     +/-   ##
=========================================
+ Coverage     68.4%   76.5%   +8.0%     
=========================================
  Files          294     300      +6     
  Lines        59390   63072   +3682     
=========================================
+ Hits         40652   48261   +7609     
+ Misses       18738   14811   -3927     
Files with missing lines Coverage Δ
flopy/mf6/data/mfdatastorage.py 75.4% <ø> (+6.5%) ⬆️
flopy/mf6/utils/codegen/__init__.py 100.0% <100.0%> (ø)
flopy/mf6/utils/codegen/context.py 100.0% <100.0%> (ø)
flopy/mf6/utils/createpackages.py 80.0% <75.0%> (+73.5%) ⬆️
flopy/mf6/utils/generate_classes.py 20.3% <83.3%> (+2.5%) ⬆️
flopy/mf6/utils/codegen/dfn.py 96.0% <96.0%> (ø)
flopy/mf6/utils/codegen/filters.py 93.5% <93.5%> (ø)
flopy/mf6/utils/codegen/dfn2toml.py 0.0% <0.0%> (ø)

... and 226 files with indirect coverage changes

@wpbonelli
Copy link
Member Author

wpbonelli commented Jan 14, 2025

outdated @deltamarnix after considering I think your work in progress (removing the legacy `dfn` attr on the generated classes) should go before this. This PR has shrunk a lot with the dfn -> toml conversion provided by devtools. It would be messy to try to preserve or reconstruct the old unstructured format when the whole point is to drop it and clean things up. So I took the `dfn` attr out of the templates in the last commit here. Let's discuss.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dependencies Pull requests that update a dependency file maintenance modflow 6
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants