Skip to content
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.

Sign up for GitHub

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

Jump to bottom

1. flickr fetcher / mappers #384

Merged
merged 21 commits into from
Aug 7, 2023
Merged

1. flickr fetcher / mappers #384

merged 21 commits into from
Aug 7, 2023

Conversation

lthurston
Copy link
Contributor

@lthurston lthurston commented Apr 6, 2023 •
edited
Loading

  • The description values with the Rikolti mappers is a modest improvement over the legacy mapper results. The source data is quite bad, resulting from using the description field as a metadata store in inconsistent ways.
  • There's an issue with validation of collection 27125 that we need to look into.

@lthurston lthurston added the WIP label Apr 6, 2023
@lthurston lthurston added this to the #4 CIC work milestone Apr 7, 2023
This was linked to issues Apr 7, 2023
FlickrMapper(Mapper) #273
Closed
FlickrSDASMMapper(FlickrMapper) #274
Closed
FlickrSPPLMapper(FlickrMapper) #275
Closed
@lthurston lthurston force-pushed the flk-mappers branch 5 times, most recently from 6bbc0fd to a3b2a42 Compare April 13, 2023 22:22
@lthurston lthurston changed the title [WIP] Implement flickr.flickr Implement flickr.flickr Apr 14, 2023
@lthurston lthurston changed the title Implement flickr.flickr Implement flickr mappers Apr 14, 2023
@lthurston
Copy link
Contributor Author

@barbarahui @amywieliczka Despite all my huffing and puffing about the sdasm mapper, I think I got it in a fairly good place. I have yet to validate against solr, and want to do that before I say this is officially ready for review. If you have time to take a look at these mappers to see if you have any questions, comments, objections, that'd be great. If not that's cool, too.

The multiple executions of the split_description() method on sdasm and sppl are not the most egregious waste of electricity / CPU cycles ever (that dubious award goes to cryptocurrency mining of course), but it does give me a minor twinge in the part of my brain that seeks efficiency. Like Elsa, I can let it go.

@lthurston lthurston force-pushed the flk-mappers branch 3 times, most recently from 8dacf3a to acc0ed2 Compare April 21, 2023 18:48
@lthurston lthurston marked this pull request as ready for review April 27, 2023 21:31
@lthurston lthurston changed the title Implement flickr mappers 2. Implement flickr mappers Apr 27, 2023
@lthurston lthurston removed the WIP label May 1, 2023
@lthurston lthurston changed the base branch from flk-fetcher to main May 8, 2023 17:08
@lthurston
Reduce calls to split_description
9d629f2 
This impacts sppl and sdasm, both child mappers
of flickr.
@lthurston
Trim whitespace around rights value
ebea5f1 
Collection 27125 has an errant space at the end of its
rights statement.
@lthurston
Wrap description in list for SPPL
00eaa4f
@lthurston
Fix function signature for aggregate_vernacular_content
d2965c1
@lthurston lthurston marked this pull request as ready for review June 8, 2023 00:01
@lthurston lthurston linked an issue Jul 26, 2023 that may be closed by this pull request
flickr.sdasm mapper validation results #460
Closed
@lthurston lthurston force-pushed the flk-mappers branch 2 times, most recently from 8915635 to 4c88b9e Compare July 26, 2023 18:04
@lthurston lthurston linked an issue Jul 26, 2023 that may be closed by this pull request
flickr.sppl mapper validation results #461
Closed
@amywieliczka
change regex for Previous Identifier; split_description overload;
cc2ecd2 
rather than copying and moving fields out of description all in
one function with finicky ordering properties, use separate
search-only and substitute-only functions
@amywieliczka
Move local call number into identifier from description
df9bfcc
@amywieliczka
Update local call number mapping; add validation exception
9c6f437
amywieliczka
amywieliczka previously approved these changes Aug 7, 2023
View reviewed changes
@amywieliczka amywieliczka merged commit 9f7273c into main Aug 7, 2023
1 check passed
@amywieliczka amywieliczka deleted the flk-mappers branch August 7, 2023 23:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@amywieliczka amywieliczka amywieliczka left review comments

@barbarahui barbarahui Awaiting requested review from barbarahui barbarahui is a code owner

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
#4 CIC work
2 participants
@lthurston @amywieliczka