-
Notifications
You must be signed in to change notification settings - Fork 34
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
encode-marcxml
should combine split leader fields of decode-marc21
to create valid marc xml
#527
Comments
Asking @blackwinter @fsteeg @hagbeck @maipet @TobiasNx : this would change the default behaviour - how should we procede: a) just fix it and mention the fix as we normally do |
In my opinion this should similar be handled as #401. It is a bug, It should not be backwards compatible. |
I would really like to go with b) which saves us (and the code) much extra work in comparison to a). |
In my opinion All leader info should already be available either since they are a) static, b) can be generated by the encoder (as #524) or c) provided by the
|
I know this all. My proposal is, if it's true that we always need
|
Talked to @dr0i and @maipet the easy solution is instead of generating the Leader Pos 00-04 and Pos 12-16 is just to set them to 0 if the leader is provided incomplete and in separated elements: That also echos voices from the literature:
from: XML for Catalogers and Metadata Librarians by Timothy W. Cole and Myung-Ja Han, https://dl.acm.org/doi/10.5555/2531760 and examples from DNB, ALMA and others: |
We discussed offline: we have to properly parse the leader, e.g. because the order of the
to this:
i.e. |
Marc21XmlEncoder acts as a wrapper. It makes use of Marc21Encoder, Marc21Decoder and MarcXmlEncoder to ensure a proper MarcXml, especially regarding the leader. Also - in contrast to MarcXmlEncoder - the record id (field 001) is mandatory.
See also #336. Co-authored-by: Pascal Christoph <pascal.christoph@hbz-nrw.de>
It seems that the initial usecase is not solved. The leader seems to come out wrong:
|
@blackwinter can you have a look? Unfortunately I've already released this buggy version. |
Also the changes introduced a new bug: https://test.metafacture.org/playground/?flux=%22https%3A//raw.githubusercontent.com/metafacture/metafacture-core/master/metafacture-runner/src/main/dist/examples/read/marc21/10.marc21%22%0A%7C+open-http%0A%7C+as-lines%0A%7C+decode-marc21%28emitLeaderAsWhole%3D%22true%22%29%0A%7C+encode-marcxml%0A%7C+print%0A%3B (this worked before)
|
These are actually the same bug. It's fixed in 32beffb. |
Absolutely! |
I tested it locally the leader is not repeated and concatenated anymore which is great!!
now
But this still outputs a wrong leader because it misses the counted elements: It should look something like: |
You haven't set |
Great: with this option this looks good!! |
+1 |
Instead of just appending the incoming leader elements, this adds the static values for Pos: 00-04, 10-16, 20-23. By using `0`as default value for Pos: 00-04 and 12-16. #527 (comment)
Instead of just appending the incoming leader elements, this adds the static values for Pos: 00-04, 10-16, 20-23. By using `0`as default value for Pos: 00-04 and 12-16. #527 (comment)
If the incoming leader is 8 Metafacture is now able to create correct leaders for marc xml with static default values.
encode-marcxml creates invalid marc data when processing
decoded-marc21
with separated leader elements.It creates invalid marc xml data. See the leader spec here in the schema: https://www.loc.gov/standards/marcxml/schema/MARC21slim.xsd
encode-marcxml
should behave in the same way asencode-marc21
, if the data is seperated the encoder should combine them.Status quo:
https://test.metafacture.org/playground/?flux=%22https%3A//raw.githubusercontent.com/metafacture/metafacture-core/master/metafacture-runner/src/main/dist/examples/read/marc21/10.marc21%22%0A%7C+open-http%0A%7C+as-lines%0A%7C+decode-marc21%0A%7C+encode-marcxml%0A%7C+print%0A%3B
decode-marc21(emitLeaderAsWhole="false")` -> encode-marcxml => the separated elements are kept:
leader should be combined.
Originally posted by @TobiasNx in #526 (comment)
The text was updated successfully, but these errors were encountered: