Incomplete inner fields

[martclanor]

Hi,

Noticed a regression where inner fields (children of other fields) are sometimes excluded during the drilldown, except for the first child.

For example, when looking at the gwf-wel component, options block, afrcsv_filerecord field:

spec.components["gwf-wel"].blocks["options"].fields["afrcsv_filerecord"].fields.keys()
# Returns dict_keys(['auto_flow_reduce_csv'])
# Should return dict_keys(['auto_flow_reduce_csv', 'fileout', 'afrcsvfile'])

I checked the v2.0.0 snapshots, and this behavior seems to have been introduced in dev2.

Also, just to add more info, this doesn't seem to be the case for some other inner fields. For example:

spec.components["gwt-ssm"].blocks["sources"].fields["sources"].item.fields.keys()
# Correctly returns all children: dict_keys(['pname', 'srctype', 'auxname'])

[martclanor]

Quick update: FWIW, while tracing through the code, I noticed that the following line,

modflow-devtools/modflow_devtools/dfns/migrate_to_v2_0_0_dev2.py

Line 1308 in adf2f1a

rec_fields = _subfield_map()

is not being called reached and is probably related to the issue on this specific case. I'm still unsure how exactly to change things around here, but I thought I'd throw this in as it might offer a clue.

[aprovost-usgs]

@martclanor - Thanks for posting the issue and providing the update. The members of our team who can best evaluate this are currently out of the office. We'll follow up with a response as soon as we can.

[martclanor]

Thanks for letting me know, no rush at all!

[martclanor]

Found another relevant case in the utl-obs component where the binary field, among others, is missing. See autotest/dfns/__snapshots__/v2.0.0.dev3/utl-obs.yaml.

I believe this one is related to the fact that this line isn't wrapped with the try_parse_bool function:

modflow-devtools/modflow_devtools/dfns/migrate_to_v2_0_0_dev2.py

Line 1366 in adf2f1a

if field.get("in_record", False):

[wpbonelli]

Thanks for this @martclanor and sorry for the delay! You're right about the try_parse_bool issue.

Re: file records collapsing down to one field, that is intentional. The tentative new DFN schema has a first-class File type, mentioned in the spec here. The idea is a cleaner type system and a (mostly) format-agnostic spec. But I see there are a few issues:

1. The File fields lose the correct longname/description. The migration pulls from the leading tag field, not the path field.
2. The file record pattern is not universally TAG MODE PATH, there can be a trailing keyword flag and/or multiple leading tags, e.g.

• utl-obs.output is MODE PATH FLAG: no tag, trailing flag
• gwf-sfr.cross_sectionrecord is TAG TAG MODE PATH, two tag tokens for a single file path

I was hoping .render() would give you exactly what your plugin needs to show for hover text for each component, block, or field. Also useful for our docs. But I realize I didn't add a .render() method to the field types yet.

I'll work on all this and comment again here when it's in better shape. In general I'm curious whether you agree with the approach. Really value your feedback.

(Note the migration scripts are almost completely agent-generated, they can be thrown away once we're happy with the new spec/schema and switch MF6 over to it, so I've been playing pretty fast/loose with them. Sorry to make you wade through that.)

[wpbonelli]

Just a heads up. While working on render() I noticed your plugin only tags keyword fields, and ignores tagged for other fields. And it looks like it renders inline records as collapsed placeholders <record name> in all cases. So for instance GWF OC formatrecord hover looks like

[HEAD PRINT_FORMAT <formatrecord>]
<columns> <width> <digits> <format>

While in the MF6IO PDF it's fully rendered inline

HEAD PRINT_FORMAT COLUMNS <columns> WIDTH <width> DIGITS <digits> <format>

I think the collapsed placeholder makes sense for unions, since showing all the possible choices would be cluttered. The IO guide and the plugin are already consistent on this. E.g. GWF OC period block

[SAVE <rtype> <ocsetting>]
[PRINT <rtype> <ocsetting>]

But for records not part of a union, I'd say a full render makes sense?