-
Notifications
You must be signed in to change notification settings - Fork 821
Add Discogs translator #3451
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
base: master
Are you sure you want to change the base?
Add Discogs translator #3451
Conversation
A Zotero translator for Discogs ( https://www.discogs.com/). Tested on https://www.discogs.com/release/126504-Micro-Vicious-Vic-Electric-Impact-The-Mixes
Still working on getting CI working again; don't worry about the errors. |
HISTORY | ||
|
||
This translator was something I've been circling arund for 3 years. I started a BA(Hons) in Creative Music Technology at Falmouth University. | ||
I became frustrated with the lack of citation tools for music releases and audio recordings in general. Zotero does detect the Discogs page as an | ||
audio recording but does not fill in the correct details. Other citation tools out there fail to have searches for music releases and seem to have | ||
no idea that vinyl records can in fact be cited and referenced. All this when as a DJ I am researching releases and writing assessments and dissertations ! | ||
So I hope this translator can improve the situation. | ||
|
||
METHOD |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Move this to the PR description! Good info but doesn't need to be in the translator code.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OK, I copied to the top of this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can be removed from the code.
|
||
METHOD | ||
|
||
This translator accesses the "ld+json" block in the "release" Discogs page. It is not designed to use the "master" page for a release as that will not |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you give an example? We need to make sure detectWeb()
doesn't match pages we can't scrape.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes ... (from https://www.discogs.com/release/1468494-The-Future-Sound-Of-London-By-Any-Other-Name )
<div class="content_f2X3g" id="page" role="main"><script type="application/ld+json" id="release_schema">{"@context":"http://schema.org","@type":"MusicRelease","@id":"https://www.discogs.com/release/1468494-The-Future-Sound-Of-London-By-Any-Other-Name","name":"By Any Other Name","musicReleaseFormat":"CD","genre":["Electronic"],"description":"FSOL Digital Recordings © 2008\r\nDistributed in the UK by Pinnacle\r\n\r\nTrack 14 does not appear on the artwork.\r\n","datePublished":2008,"catalogNumber":"CD TOT 61, CDTOT61","recordLabel":[{"@type":"Organization","@id":"https://www.discogs.com/label/84706-fsoldigital.com","name":"fsoldigital.com"}],"releaseOf":{"@type":"MusicAlbum","@id":"https://www.discogs.com/master/view/413607","name":"By Any Other Name","datePublished":2008,"byArtist":[{"@type":"MusicGroup","name":"The Future Sound Of London","@id":"https://www.discogs.com/artist/2549-The-Future-Sound-Of-London"}]},"releasedEvent":{"@type":"PublicationEvent","startDate":2008,"location":{"@type":"Country","name":"UK"}},"image":"https://i.discogs.com/nZHPwrPZMUSiJScg9d5-WyAxiPoxPegiqoGaMjNuuw4/rs:fit/g:sm/q:90/h:597/w:600/czM6Ly9kaXNjb2dz/LWRhdGFiYXNlLWlt/YWdlcy9SLTE0Njg0/OTQtMTMyMzA1NjY1/OS5qcGVn.jpeg","offers":{"@type":"AggregateOffer","offerCount":6,"priceCurrency":"USD","lowPrice":5.62,"itemOffered":{"@type":"Product","name":"The Future Sound Of London - By Any Other Name","aggregateRating":{"@type":"AggregateRating","ratingCount":58,"ratingValue":4.38}}}}</script>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OK, but we need to make sure it doesn't match pages like this, then. Right now it seems like it does.
} | ||
|
||
function doWeb(doc, url) { | ||
// Create a new Zotero item of type "audioRecording" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Run npm run lint -- --fix Discogs.js
to normalize indentation.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I get the same lint error your CI is getting. This is when running that command in my cloned translators folder with recommended npm install.
// Complete the item and save it to Zotero | ||
item.complete(); | ||
} catch (e) { | ||
Zotero.debug("Error parsing JSON-LD: " + e.message); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Don't wrap in a try-catch - just let it fail.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just item.complete(); then ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not sure what you mean - just replace the entire try-catch with the try
block's contents.
} | ||
|
||
/** BEGIN TEST CASES **/ | ||
var testCases = [ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add test cases using Scaffold
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was having trouble understanding how to do that. Do they need to be there ? I can fix anything that will prevent the PR succeeding but have limited time beyond that.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, we definitely need test cases. Open one of the example pages in the Browser tab and click "Create Web Test," then save the translator.
A Zotero translator for Discogs ( https://www.discogs.com/). Tested on https://www.discogs.com/release/126504-Micro-Vicious-Vic-Electric-Impact-The-Mixes
HISTORY
This translator was something I've been circling around for 3 years. I started a BA(Hons) in Creative Music Technology at Falmouth University. I became frustrated with the lack of citation tools for music releases and audio recordings in general. Zotero does detect the Discogs page as an audio recording but does not fill in the correct details. Other citation tools out there fail to have searches for music releases and seem to have no idea that vinyl records can in fact be cited and referenced. All this when as a DJ I am researching releases and writing assessments and dissertations ! So I hope this translator can improve the situation.
METHOD
This translator accesses the "ld+json" block in the "release" Discogs page. It is not designed to use the "master" page for a release as that will not list the record label.
INFO
Some comments in the code are based on the page https://www.discogs.com/release/126504-Micro-Vicious-Vic-Electric-Impact-The-Mixes