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

Add labels of complex subject #667

Merged
merged 2 commits into from Jan 12, 2018

Conversation

Projects
None yet
3 participants
@ChristophEwertowski
Contributor

ChristophEwertowski commented Jan 9, 2018

BT000040377 is still incorrect but will be dealt with in a different ticket because it's a fringe case.

Resolves #639

Add labels of complex subject
Still incorrect: BT000040377

See #639
@ChristophEwertowski

This comment has been minimized.

Show comment
Hide comment
@ChristophEwertowski

ChristophEwertowski Jan 9, 2018

Contributor

The second example is an old NWBib subject which originally was "Schulgeschichte / Herne" but is splitted in Aleph in two subfields a. To cite Mrs. Blankenhagen:

Da Unterfeld a nicht wiederholbar ist, wurde für den WorldCat-Export eine eigene Vorgabe geschrieben:
Bei Vorhandensein von mehreren $a soll der Inhalt des zweiten und weiteren $a jeweils nach Komma Blank (,_) an den Inhalt des ersten $a angefügt werden (betr. MARC 65X und 689).

Because "/" could be misinterpretated as a separator I stick to the WorldCat convention.

@fsteeg : Tried to run it at test.lobid.org but the java.logs says JNA not found. Either it get's fixed or you look only at the diff.

Contributor

ChristophEwertowski commented Jan 9, 2018

The second example is an old NWBib subject which originally was "Schulgeschichte / Herne" but is splitted in Aleph in two subfields a. To cite Mrs. Blankenhagen:

Da Unterfeld a nicht wiederholbar ist, wurde für den WorldCat-Export eine eigene Vorgabe geschrieben:
Bei Vorhandensein von mehreren $a soll der Inhalt des zweiten und weiteren $a jeweils nach Komma Blank (,_) an den Inhalt des ersten $a angefügt werden (betr. MARC 65X und 689).

Because "/" could be misinterpretated as a separator I stick to the WorldCat convention.

@fsteeg : Tried to run it at test.lobid.org but the java.logs says JNA not found. Either it get's fixed or you look only at the diff.

@ChristophEwertowski ChristophEwertowski requested a review from fsteeg Jan 9, 2018

@fsteeg

This comment has been minimized.

Show comment
Hide comment
@fsteeg

fsteeg Jan 10, 2018

Contributor

Test system is now running again (the proxy was still configured to use quaoar1, not weywot2):

http://test.lobid.org/resources/BT000003404
http://test.lobid.org/resources/BT000103077
http://test.lobid.org/resources/search

I'll look into the naming issue mentioned in #639 (comment) and will then review this.

Contributor

fsteeg commented Jan 10, 2018

Test system is now running again (the proxy was still configured to use quaoar1, not weywot2):

http://test.lobid.org/resources/BT000003404
http://test.lobid.org/resources/BT000103077
http://test.lobid.org/resources/search

I'll look into the naming issue mentioned in #639 (comment) and will then review this.

@fsteeg

This comment has been minimized.

Show comment
Hide comment
@fsteeg

fsteeg Jan 10, 2018

Contributor

I think there should be a way to make the issue mentioned in #639 (comment) work. (Re-)using subject.label (until now only used by for non-componentList subjects like the NWBib classification) will make the aggregation more involved (we only want the values where the type is ComplexSubject, we'll probably have to combine a nested aggregation and a bucket selector aggregation), but it's nice that we'll get a single subject.label field which can also be used for queries against all subjects (both with and without componentList).

So +1 from me on this. If it turns out this does not work, we can still rename the field to something specific, since we don't take any field away (subject.label already exists). Reassigning @dr0i for additional review and subsequent deployment.

Contributor

fsteeg commented Jan 10, 2018

I think there should be a way to make the issue mentioned in #639 (comment) work. (Re-)using subject.label (until now only used by for non-componentList subjects like the NWBib classification) will make the aggregation more involved (we only want the values where the type is ComplexSubject, we'll probably have to combine a nested aggregation and a bucket selector aggregation), but it's nice that we'll get a single subject.label field which can also be used for queries against all subjects (both with and without componentList).

So +1 from me on this. If it turns out this does not work, we can still rename the field to something specific, since we don't take any field away (subject.label already exists). Reassigning @dr0i for additional review and subsequent deployment.

@fsteeg fsteeg requested review from dr0i and removed request for fsteeg Jan 10, 2018

@dr0i

dr0i approved these changes Jan 11, 2018

@dr0i dr0i added deploy and removed review labels Jan 11, 2018

@dr0i dr0i merged commit 43e40f7 into master Jan 12, 2018

2 checks passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details

@dr0i dr0i removed the deploy label Jan 12, 2018

@dr0i dr0i referenced this pull request Jan 16, 2018

Closed

Add labels to ComplexSubjects #639

@dr0i

This comment has been minimized.

Show comment
Hide comment
@dr0i

dr0i Jan 16, 2018

Contributor

Deployed to prodcution, closed.

Contributor

dr0i commented Jan 16, 2018

Deployed to prodcution, closed.

@dr0i dr0i deleted the 639-addLabelToComplexSubjects branch Jan 16, 2018

@dr0i dr0i removed their assignment Jan 18, 2018

@dr0i dr0i requested review from dr0i and removed request for dr0i Jan 18, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment