BUG: Lots of element descriptions are (incorrectly) generic (v2.03 only)

[andylolz]

At v2.02, the location/name definition was:

The human-readable name for the location.

At v2.03, the location/name definition is:

Data type for an element that must contain human-readable text. The information may be repeated in different languages.

This generic definition is significantly less useful. I guess this relates to #199 / #200 / DRYing up the schema at v2.03.

[andylolz]

I ran into this when trying to point a publisher to the location/name documentation, in order to explain how it should be used. The definition in the v2.03 documentation doesn’t answer this.

It seems likely there are various other instances of this same issue in the v2.03 documentation.

[andylolz]

I took a look at the cause of the issue, and I can see it’s a problem in lots of places.

The definition for all of the following elements is the same:

• iati-activity/title
• iati-activity/contact-info/organisation
• iati-activity/contact-info/department
• iati-activity/contact-info/person-name
• iati-activity/contact-info/job-title
• iati-activity/contact-info/mailing-address
• iati-activity/transaction/description
• iati-activity/location/name
• iati-activity/location/description
• iati-activity/location/activity-description
• iati-activity/country-budget-items/budget-item/description

It’s the very generic:

Data type for an element that must contain human-readable text. The information may be repeated in different languages.

[samuele-mattiuzzo]

Thanks Andy, we'll look into this

[andylolz]

Great – thanks @samuele-mattiuzzo.

Other definitions that have changed for the worse:

• transaction/value – definition was previously:

  The amount of the contribution.

  and is now:

  Data type for an element containing a currency value.

• iati-activity/transaction/aid-type – definition was previously:

  Optional element to override the top-level default-aid-type element (debt relief, etc.) on a transaction-by-transaction basis if needed.

  and is now:

  A code from the specified vocabulary.

…etc. So, everywhere a custom type is used. I guess generalising the solution in #210 could solve the issue?

[samuele-mattiuzzo]

After removing the code added in #210 , it seems like the definitions still remain "bugged"

This needs further investigation, as that didn't seem to have caused this problem

[andylolz]

After removing the code added in #210 , it seems like the definitions still remain "bugged"

Oh sorry, I didn’t mean to suggest that #210 caused this. This has been a bug since the start of v2.03, I think, when this extra hierarchy was introduced.

All I meant was… I was speculating that #210 looks document-link-specific, but could perhaps be generalised to other elements somehow. But I haven’t looked into this – this is just from looking at the diff, so @Ocre42 will have a far better idea than me on this.

[samuele-mattiuzzo]

Ah right, makes sense (I think?)! Will try to fix it extending John's code for now, then maybe during the TAG we can have a mini-sprint if we get the time and sort the Schema or Schema2Doc together

[andylolz]

then maybe during the TAG we can have a mini-sprint if we get the time and sort the Schema or Schema2Doc together

YAS!

⛵ I’m on board ⛵

[andylolz]

I haven’t quite gotten to the bottom of this yet… But I think the issue is with the following bit of code:

IATI-Standard-SSOT/gen.py

Lines 218 to 231 in 7c1fb99

	if type_element is not None:
	if(element.get("name") == "document-link" and type_element.find(".//xsd:extension", namespaces=namespaces) is not None):
	base_name = type_element.find(".//xsd:extension", namespaces=namespaces).get("base")
	base_element = self.tree2.find("xsd:{0}[@name='{1}']".format("complexType", base_name), namespaces=namespaces)
	return base_element.find(".//xsd:documentation", namespaces=namespaces).text
	xsd_documentation = type_element.find(".//xsd:documentation", namespaces=namespaces)
	if xsd_documentation is not None:
	return type_element.find(".//xsd:documentation", namespaces=namespaces).text
	if ref_element is not None:
	xsd_docuementation = ref_element.find(".//xsd:documentation", namespaces=namespaces)
	if xsd_docuementation is not None:
	return ref_element.find(".//xsd:documentation", namespaces=namespaces).text
	return element.find(".//xsd:documentation", namespaces=namespaces).text

This defines an order of precedence for which documentation to use for an element. The order is defined as the following:

base type element >> type element >> ref element >> element

I think this order of precedence is backwards, and I think that’s the issue here. If there’s documentation on an element, that should be used preferentially. The documentation on the ref or type element should be used as a fallback. The documentation on the base type element should be used as a fallback for the type element.

Not totally sure, but I think this bug was introduced in #125… In conjunction with schema changes introduced at v2.03.

[andylolz]

I’ve sent fixes for all v2.0x branches.

As for #200, I haven’t sent fixes for v1.0x branches, because the code there is quite stale and out of sync so it’s non-trivial to add the same update. In any case, it wouldn’t affect the output, since this bug only exists at v2.03 anyway.

---

I flagged a number of times with the previous tech team that their pull request review process didn’t seem sufficiently rigorous. Specifically, it seemed as if the code was eyeballed, but not actually run.

This bug seems symptomatic of this insufficiently thorough review process. If the reviewer of #125 had compared the output before and after the PR, I’m certain they would have spotted this bug, since it’s pretty big (see the diff in #241 (comment)). I really hope with the change in personnel, issues with this process will be addressed.

[andylolz]

Looks good!