Opened 13 years ago

Closed 13 years ago

Last modified 13 years ago

#1230 closed defect (fixed)

<suppress_required/> in r.out.gdal

Reported by: rsbivand Owned by: grass-dev@…
Priority: major Milestone: 7.0.0
Component: Raster Version: svn-trunk
Keywords: Cc:
CPU: Unspecified Platform: Unspecified

Description

r.out.gdal --interface-description

now yields:

...

<flag name="l">

<suppress_required/> <description>

List supported output formats

</description> <guisection>

Print

</guisection>

</flag>

...

which breaks the spgrass6 R interface. Is this well-formed XML?

g.version

GRASS 7.0.svn44543 (2010)

Change History (7)

comment:1 by hamish, 13 years ago

<suppress_required/> is shorthand for <suppress_required></suppress_required>, and is valid XML.

Hamish

comment:2 by rsbivand, 13 years ago

It may be permitted, but is much less obvious than

<suppress_required>yes</suppress_required> <suppress_required>no</suppress_required>

which is arguably more portable.

r.in.gdal is affected by the same non-portability in flag "f".

I have to work within the XML library linked by the XML R contributed package, so if that barfs, I'm stuck. One problem seems to be that <description> has typically always been the first component, but where <suppress_required> appears, it is now first. So some modules will optionally get an out-of-order component which, when present, means yes, and when absent means no? I guess I'm going to have to add code to loop through each flag entry checking the XML names, and set yes/no myself.

Is the logic here that if I say "r.in.gdal -f", and if suppress_required is not set, one of the parameters may apply its required=YES value to terminate execution? Can flags with and without suppress_required set be mixed?

comment:3 by rsbivand, 13 years ago

Resolution: fixed
Status: newclosed

Resolved by trapping "suppress_required" in R code, and storing it to use in checking the required status of parameters in constructing the command string. It will be picked up again within GRASS when the RUI passes the completed command string through system(), but the command string ought to pass OK. The problem was my expecting the description component in flag objects to come first, which may not be the case when suppress_required is used. spgrass6_0.6-23.tar.gz submitted to CRAN.

in reply to:  2 ; comment:4 by hamish, 13 years ago

Replying to rsbivand:

I have to work within the XML library linked by the XML R contributed package, so if that barfs, I'm stuck.

If R's XML package can't deal with <foo />, then we should probably file a bug with them. While not in every XML file it's a somewhat common creature and I'd be a little surprised if you were the first person to find this problem, and you certainly wont be the last.

Hamish

ps- is the missing newline after the tag real or just an artifact of the trac editor?

in reply to:  2 ; comment:5 by glynn, 13 years ago

Replying to rsbivand:

It may be permitted, but is much less obvious than

<suppress_required>yes</suppress_required> <suppress_required>no</suppress_required>

which is arguably more portable.

XML's support for the abbreviation of empty tags isn't an optional feature. If a parser doesn't handle it, it isn't an XML parser, it's a parser for an entirely different language.

I have to work within the XML library linked by the XML R contributed package, so if that barfs, I'm stuck. One problem seems to be that <description> has typically always been the first component, but where <suppress_required> appears, it is now first.

I note that the --interface-description output doesn't conform to the stated DTD. But that was true before the suppress_required element was added (e.g. the DTD doesn't list "guisection" elements as being valid children for "flag" elements), so I'm guessing that this isn't actually the problem.

I'm willing to fix both the DTD and the --interface-description output so that they match, although someone (probably Martin) will need to update the wx GUI. Based upon existing usage, I think that suppress_required may be better off as an attribute rather than a child element. OTOH, the split between which fields are attributes and which are child elements is rather arbitrary.

So some modules will optionally get an out-of-order component which, when present, means yes, and when absent means no? I guess I'm going to have to add code to loop through each flag entry checking the XML names, and set yes/no myself.

You should be doing that anyhow. Most of the fields are marked as optional according to the DTD, so you can't assume that e.g. the first child is a description element.

You can just ignore the suppress_required element if you aren't checking requirements. The wx GUI does check requirements, so that element had to be included in the interface description to prevent it from complaining (unnecessarily) about missing options.

Is the logic here that if I say "r.in.gdal -f", and if suppress_required is not set, one of the parameters may apply its required=YES value to terminate execution?

Yes. Previously, if a module had a "special case" flag which invoked a different mode of execution, all of its options had to be listed as ->required=NO. The addition of the suppress_required fields allows options' ->required fields to be set based upon "normal" usage, rather than "worst-case" usage.

Can flags with and without suppress_required set be mixed?

Yes. The only effect of that field is that if a flag with the suppress_required field is given, G_parser() suppresses the normal check that any options with ->required=YES are actually given.

in reply to:  4 comment:6 by rsbivand, 13 years ago

Replying to hamish:

Replying to rsbivand:

I have to work within the XML library linked by the XML R contributed package, so if that barfs, I'm stuck.

If R's XML package can't deal with <foo />, then we should probably file a bug with them. While not in every XML file it's a somewhat common creature and I'd be a little surprised if you were the first person to find this problem, and you certainly wont be the last.

Hamish

ps- is the missing newline after the tag real or just an artifact of the trac editor?

Hamish: the trac editor, in a gnome terminal there is a newline between <suppress_required/> and <description>, both are equally indented.

in reply to:  5 comment:7 by rsbivand, 13 years ago

Replying to glynn:

Replying to rsbivand:

It may be permitted, but is much less obvious than

<suppress_required>yes</suppress_required> <suppress_required>no</suppress_required>

which is arguably more portable.

XML's support for the abbreviation of empty tags isn't an optional feature. If a parser doesn't handle it, it isn't an XML parser, it's a parser for an entirely different language.

I have to work within the XML library linked by the XML R contributed package, so if that barfs, I'm stuck. One problem seems to be that <description> has typically always been the first component, but where <suppress_required> appears, it is now first.

I note that the --interface-description output doesn't conform to the stated DTD. But that was true before the suppress_required element was added (e.g. the DTD doesn't list "guisection" elements as being valid children for "flag" elements), so I'm guessing that this isn't actually the problem.

I'm willing to fix both the DTD and the --interface-description output so that they match, although someone (probably Martin) will need to update the wx GUI. Based upon existing usage, I think that suppress_required may be better off as an attribute rather than a child element. OTOH, the split between which fields are attributes and which are child elements is rather arbitrary.

Glynn,

Thanks, I understand now. In the revision reported above, I look at the child names, and set a value to TRUE for each R flag object if "suppress_required" is among the names, otherwise FALSE.

So some modules will optionally get an out-of-order component which, when present, means yes, and when absent means no? I guess I'm going to have to add code to loop through each flag entry checking the XML names, and set yes/no myself.

You should be doing that anyhow. Most of the fields are marked as optional according to the DTD, so you can't assume that e.g. the first child is a description element.

No, they just always had been before, my mistake.

You can just ignore the suppress_required element if you aren't checking requirements. The wx GUI does check requirements, so that element had to be included in the interface description to prevent it from complaining (unnecessarily) about missing options.

I am checking requirements.

Is the logic here that if I say "r.in.gdal -f", and if suppress_required is not set, one of the parameters may apply its required=YES value to terminate execution?

Yes. Previously, if a module had a "special case" flag which invoked a different mode of execution, all of its options had to be listed as ->required=NO. The addition of the suppress_required fields allows options' ->required fields to be set based upon "normal" usage, rather than "worst-case" usage.

Can flags with and without suppress_required set be mixed?

Yes. The only effect of that field is that if a flag with the suppress_required field is given, G_parser() suppresses the normal check that any options with ->required=YES are actually given.

Thanks. That seems clear enough - note that I closed this ticket once I'd realised that the problem was on my side.

Note: See TracTickets for help on using tickets.