srophe / syriaca-data

Repository for Syriaca.org TEI data, used by srophe-eXist-app.
4 stars 16 forks source link

Changes to note type=”deprecation” #766

Closed davidamichelson closed 4 years ago

davidamichelson commented 5 years ago

<note type=”deprecation”> Create @target with a value that concatenates ‘#’ and the substring before ‘-deprecation’ of the current xml:id Delete the xml:id Delete any <link> element that has a type value that includes the string “deprecation”

davidamichelson commented 4 years ago

@dlschwartz We might need a refresher on this one. Is this what we want?

Current:

                    <note type="deprecation" xml:id="name993-3-deprecation">Dolabani transcribed the name from Arabic rather than presenting a native Syriac form.</note>
                    <link target="#name993-3 #name993-3-deprecation"/>
                    <link target="#name993-4 #name993-3-deprecation"/>

Change:

                    <note type="deprecation" xml:id="name993-3-deprecation" target="#name993-3 #name993-4">Dolabani transcribed the name from Arabic rather than presenting a native Syriac form.</note>
davidamichelson commented 4 years ago

In other words our goal was to simplify?

wlpotter commented 4 years ago

First pass at this script is here: https://github.com/wlpotter/miscellaneous-util-scripts/blob/master/Syriaca-Places-Schema-Testing-Util/update-deprecation-note-links.xql

wlpotter commented 4 years ago

note to self: run on place data for now; may adapt script later to run data-wide

wlpotter commented 4 years ago

Before I make this change, I wanted to clarify that the output is right.

See the comment above. Did we decide that this is the change we want to make?

dlschwartz commented 4 years ago

@wlpotter @davidamichelson yes, I believe this is a more streamlined way to handle deprecation.

wlpotter commented 4 years ago

This has been fixed for place data.

wlpotter commented 4 years ago

reopening this because I found 4 cases where tei:note[@type="deprecation"] elements have an@xml:id` attribute but are being flagged by the schematron rules.

This is the error message: "A <note> element of @type ="deprecation" should not have an @xml:id attribute."

Are we wanting to remove these xml:ids?

wlpotter commented 4 years ago

There is a seemingly related issue with the schematron rule where, on place 62, I get the following error:

Acceptable values for the @xml:id attribute on a <note> element of @type="deprecation" include: #name62-1 | #name62-2 | #name62-3 | #name62-4 | #name62-5 | #name62-6 | #name62-7 | #name62-8 | #name62-9 | #name62-10 | #name62-11 | #name62-12 | #name62-13 | #name62-14 | #name62-15.

dlschwartz commented 4 years ago

@wlpotter are you working on your local machine? Place 62 is associated with syriaca-tei-main.rnc on both dev and production. Maybe you're working in a branch? If you are in a branch that is synced to GitHub, could you send me links to the lines of the files on GitHub that throw errors? That would help a lot. I need to be able to test stuff in order to figure out what errors I may have written into the rules. Thanks.

And just so I'm clear, we do want to require @xml:id on <note type="deprecation", right?

wlpotter commented 4 years ago

@dlschwartz Sorry about the confusion. I ran a batch validation on the whole of the places directory (on this branch) using the schema and embedded schematron rules from here and have been working through those to see which need to become issues and which I can change by hand.

When I check records I've just been spot-replacing the schema association since we haven't batch updated to the one in draft-data. We should think about updating the currently associated schema, but given what you said in the last paragraph here, maybe that should wait until things are closer to being finalized?

Here is the line in place 62: https://github.com/srophe/srophe-app-data/blob/2019-May-Batch-Changes/data/places/tei/62.xml#L137. It is giving both of the errors I mentioned above.

@davidamichelson we currently are using xml:ids on all deprecation notes; is this what we want?

dlschwartz commented 4 years ago

@wlpotter thanks for the clarification. I'd like to see us associate the draft data schema as soon as possible but I guess there's no need to rush it.

Somehow I missed that I had both the requirement for a particular set of values on the @xml:id and a rule saying that there couldn't be an @xml:id. Okay, I'll change the rule disallowing an @xml:id to one requiring an @xml:id. That's easy.

I haven't written the rules on the target attribute to accommodate multiple values. That's why it throws an error but the error message indicates acceptable values that are present on the attribute. I can change that as well.

dlschwartz commented 4 years ago

@wlpotter the commit above should solve this. An @xml:id is required and multiple values are allowed on @target but each one must validate against one of the values coming from the @xml:id on the name variants in the file. Go ahead close if this seems to be working as expected.

wlpotter commented 4 years ago

Yes, this looks to be fixed. Thanks!

dlschwartz commented 4 years ago

@wlpotter @davidamichelson I found a but in the value of the xml:id so I'm reopening. In theory, the xml:id on notes deprecating names should include a reference to the name being deprecated. However, we allow multiple values on target and therefore multiple names being deprecated with a single <note type="deprecation">. See the example labeled "change" above: https://github.com/srophe/srophe-app-data/issues/766#issuecomment-627612608.

What value do we want on this xml:id?

davidamichelson commented 4 years ago

I think the xml-id for the note is now obsolete. So we can just remove it/make it optional in the schema. @wlpotter will find and delete all of them on note/@type="deprecation"

dlschwartz commented 4 years ago

@wlpotter @davidamichelson great, I agree that we really do not need this. At least, I'm hard pressed to think of a time when someone would want to point to one of these deprecation notes. I'll go ahead and remove @xml:id on note type deprecation from the schema.

dlschwartz commented 4 years ago

The schema no longer allows @xml:id on <note>. Please test and close.

wlpotter commented 4 years ago

Data has been updated. @davidamichelson will update the documentation.

davidamichelson commented 4 years ago

Documentation will note that we removed xml ids for the ntoe and the link elements and just use @target

davidamichelson commented 4 years ago

Turns out I did this already.