-
-
Notifications
You must be signed in to change notification settings - Fork 2.9k
Add support for automatic ICORE conference ranking lookup [#13476] #13699
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: main
Are you sure you want to change the base?
Changes from 3 commits
d578f69
022bcd4
5bacb29
e035e0e
3a36afe
a7b1b94
33ed0db
806309e
5330632
b092aa9
52d3acd
75d2b47
45a1d10
290cf6c
93da09f
cbd4dda
2a1a73e
6747ad1
ff5c144
89c6600
c63e2da
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,24 @@ | ||
package org.jabref.logic.icore; | ||
|
||
import java.util.Optional; | ||
import java.util.regex.Matcher; | ||
import java.util.regex.Pattern; | ||
|
||
public class ConferenceAcronymExtractor { | ||
// Regex that'll extract the string within the first deepest set of parentheses | ||
// A slight modification of: https://stackoverflow.com/a/17759264 | ||
private static final Pattern PATTERN = Pattern.compile("\\(([^()]*)\\)"); | ||
|
||
public static Optional<String> extract(String input) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Method lacks input validation for null parameter which could lead to NullPointerException. While Optional return is good, the input should be validated. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Please add JavaDoc. |
||
Matcher matcher = PATTERN.matcher(input); | ||
|
||
if (matcher.find()) { | ||
String match = matcher.group(1).strip(); | ||
if (!match.isEmpty()) { | ||
return Optional.of(match); | ||
} | ||
} | ||
|
||
return Optional.empty(); | ||
} | ||
} |
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
@@ -0,0 +1,138 @@ | ||||||
package org.jabref.logic.icore; | ||||||
|
||||||
import java.io.BufferedReader; | ||||||
import java.io.IOException; | ||||||
import java.io.InputStream; | ||||||
import java.io.InputStreamReader; | ||||||
import java.util.HashMap; | ||||||
import java.util.Map; | ||||||
import java.util.Optional; | ||||||
|
||||||
import org.jabref.logic.JabRefException; | ||||||
import org.jabref.logic.util.strings.StringSimilarity; | ||||||
import org.jabref.model.icore.ConferenceEntry; | ||||||
|
||||||
import org.apache.commons.csv.CSVFormat; | ||||||
import org.apache.commons.csv.CSVRecord; | ||||||
import org.slf4j.Logger; | ||||||
import org.slf4j.LoggerFactory; | ||||||
|
||||||
/** | ||||||
* A Repository that loads and stores the latest ICORE Conference Ranking Data and allows lookups using a conference's | ||||||
* acronym or title. | ||||||
* <p> | ||||||
* The ranking data is sourced from <a href="https://portal.core.edu.au/conf-ranks/">the ICORE Conference Ranking Portal</a>. | ||||||
* Since the website does not expose an API endpoint to fetch this data programmatically, it must be manually exported | ||||||
* from the website and stored as a resource. This means that when new ranking data is released, the old data must be | ||||||
* replaced and the <code>ICORE_RANK_DATA_FILE</code> variable must be modified to point to the new data file. | ||||||
*/ | ||||||
public class ConferenceRepository { | ||||||
private static final Logger LOGGER = LoggerFactory.getLogger(ConferenceRepository.class); | ||||||
private static final String ICORE_RANK_DATA_FILE = "/icore/ICORE2023.csv"; | ||||||
|
||||||
private final Map<String, ConferenceEntry> acronymToConference = new HashMap<>(); | ||||||
private final Map<String, ConferenceEntry> titleToConference = new HashMap<>(); | ||||||
|
||||||
public ConferenceRepository() throws JabRefException { | ||||||
InputStream inputStream = getClass().getResourceAsStream(ICORE_RANK_DATA_FILE); | ||||||
|
||||||
if (inputStream == null) { | ||||||
throw new JabRefException("ICORE rank data file not found in resources"); | ||||||
} | ||||||
|
||||||
loadConferenceDataFromInputStream(inputStream); | ||||||
} | ||||||
|
||||||
// Constructor to allow loading in test data | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
JavaDoc in MArkdown is |
||||||
public ConferenceRepository(InputStream testFileInputStream) throws JabRefException { | ||||||
loadConferenceDataFromInputStream(testFileInputStream); | ||||||
} | ||||||
|
||||||
private void loadConferenceDataFromInputStream(InputStream inputStream) throws JabRefException { | ||||||
BufferedReader reader = new BufferedReader(new InputStreamReader(inputStream)); | ||||||
|
||||||
try (inputStream; reader) { | ||||||
Iterable<CSVRecord> records = CSVFormat.DEFAULT.builder() | ||||||
.setHeader() | ||||||
.setSkipHeaderRecord(true) | ||||||
.get() | ||||||
.parse(reader); | ||||||
|
||||||
for (CSVRecord record : records) { | ||||||
String id = record.get("Id").strip(); | ||||||
String title = record.get("Title").strip().toLowerCase(); | ||||||
String acronym = record.get("Acronym").strip().toUpperCase(); | ||||||
String rank = record.get("Rank").strip(); | ||||||
|
||||||
if (id.isEmpty() || title.isEmpty() || acronym.isEmpty() || rank.isEmpty()) { | ||||||
LOGGER.warn("Missing fields in row in ICORE rank data: {}", record); | ||||||
continue; | ||||||
} | ||||||
|
||||||
ConferenceEntry conferenceEntry = new ConferenceEntry(id, title, acronym, rank); | ||||||
acronymToConference.put(acronym, conferenceEntry); | ||||||
titleToConference.put(title, conferenceEntry); | ||||||
} | ||||||
} catch (IOException e) { | ||||||
throw new JabRefException("I/O Error while reading ICORE data from resource", e); | ||||||
} | ||||||
} | ||||||
|
||||||
public Optional<ConferenceEntry> getConferenceFromAcronym(String acronym) { | ||||||
String query = acronym.strip().toUpperCase(); | ||||||
|
||||||
ConferenceEntry conference = acronymToConference.get(query); | ||||||
|
||||||
if (conference == null) { | ||||||
return Optional.empty(); | ||||||
} | ||||||
|
||||||
return Optional.of(conference); | ||||||
} | ||||||
|
||||||
public Optional<ConferenceEntry> getConferenceFromBookTitle(String bookTitle) { | ||||||
String query = bookTitle.strip().toLowerCase(); | ||||||
|
||||||
// Lucky path | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Comment does not add new information and can be plainly derived from the code. It should be removed as it doesn't provide additional context or reasoning. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||
ConferenceEntry conference = titleToConference.get(query); | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Comment 'Lucky path' doesn't add any new information and can be derived from the code itself. Comments should provide additional context or reasoning. |
||||||
if (conference != null) { | ||||||
return Optional.of(conference); | ||||||
} | ||||||
|
||||||
String bestMatch = fuzzySearchConferenceTitles(query); | ||||||
|
||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||
if (bestMatch.isEmpty()) { | ||||||
return Optional.empty(); | ||||||
} | ||||||
|
||||||
conference = titleToConference.get(bestMatch); | ||||||
|
||||||
return Optional.of(conference); | ||||||
} | ||||||
|
||||||
/** | ||||||
* Searches all conference titles for the given query string using {@link StringSimilarity#similarity} as a matcher. | ||||||
* <p> | ||||||
* The threshold for matching is set at 0.9. This function will always return the conference title with the highest | ||||||
* similarity rating. | ||||||
* | ||||||
* @param query The query string to be searched | ||||||
* @return The conference title, if found. Otherwise, an empty string is returned. | ||||||
*/ | ||||||
private String fuzzySearchConferenceTitles(String query) { | ||||||
String bestMatch = ""; | ||||||
double bestSimilarity = 0.0; | ||||||
final double FUZZY_SEARCH_THRESHOLD = 0.9; | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Extract to |
||||||
StringSimilarity matcher = new StringSimilarity(); | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Maybe, this can also be a |
||||||
|
||||||
for (String conferenceTitle : titleToConference.keySet()) { | ||||||
double similarity = matcher.similarity(query, conferenceTitle); | ||||||
if (similarity >= FUZZY_SEARCH_THRESHOLD && similarity > bestSimilarity) { | ||||||
bestMatch = conferenceTitle; | ||||||
bestSimilarity = similarity; | ||||||
} | ||||||
} | ||||||
|
||||||
return bestMatch; | ||||||
} | ||||||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -3,8 +3,12 @@ | |
import java.util.Locale; | ||
|
||
import info.debatty.java.stringsimilarity.Levenshtein; | ||
import org.slf4j.Logger; | ||
import org.slf4j.LoggerFactory; | ||
|
||
public class StringSimilarity { | ||
private static final Logger LOGGER = LoggerFactory.getLogger(StringSimilarity.class); | ||
|
||
private final Levenshtein METRIC_DISTANCE = new Levenshtein(); | ||
// edit distance threshold for entry title comparison | ||
private final int METRIC_THRESHOLD = 4; | ||
|
@@ -24,4 +28,31 @@ public double editDistanceIgnoreCase(String a, String b) { | |
// TODO: Locale is dependent on the language of the strings. English is a good denominator. | ||
return METRIC_DISTANCE.distance(a.toLowerCase(Locale.ENGLISH), b.toLowerCase(Locale.ENGLISH)); | ||
} | ||
|
||
/** | ||
* Calculates the similarity (a number within 0 and 1) between two strings. | ||
* http://stackoverflow.com/questions/955110/similarity-string-comparison-in-java | ||
*/ | ||
public double similarity(final String first, final String second) { | ||
final String longer; | ||
final String shorter; | ||
|
||
if (first.length() < second.length()) { | ||
longer = second; | ||
shorter = first; | ||
} else { | ||
longer = first; | ||
shorter = second; | ||
} | ||
|
||
final int longerLength = longer.length(); | ||
// both strings are zero length | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This comment is trivial and can be derived directly from the code condition (longerLength == 0). It should be removed as it doesn't add new information. |
||
if (longerLength == 0) { | ||
Comment on lines
+49
to
+50
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Comment is redundant as it simply restates what the code clearly shows. The comment doesn't provide additional information or reasoning. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The comment was added there by the original author of the code, I've merely ported it with the necessary modifications. That said, I will contest this review as the lines final int longerLength = longer.length();
// both strings are zero length
if (longerLength == 0) {
return 1.0;
} do not explicitly state, on their own, that the two input strings are equal. Further, the comment is present in the original Stack Overflow post here. That being said, if you're adamant about it, I don't mind changing this. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Take the bot meanings with care, they are not always that good |
||
return 1.0; | ||
} | ||
final double distanceIgnoredCase = editDistanceIgnoreCase(longer, shorter); | ||
final double similarity = (longerLength - distanceIgnoredCase) / longerLength; | ||
LOGGER.trace("Longer string: {} Shorter string: {} Similarity: {}", longer, shorter, similarity); | ||
return similarity; | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,17 @@ | ||
package org.jabref.model.icore; | ||
|
||
/** | ||
* A Conference Entry built from a subset of fields in the ICORE Ranking data | ||
*/ | ||
Comment on lines
+3
to
+5
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The comment merely restates what is obvious from the code and doesn't provide additional information about the purpose or constraints of the record. |
||
public record ConferenceEntry( | ||
String id, | ||
String title, | ||
String acronym, | ||
String rank | ||
) { | ||
private final static String URL_PREFIX = "https://portal.core.edu.au/conf-ranks/"; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Incorrect order of modifiers. According to Java conventions and effective Java principles, it should be 'private static final' instead of 'private final static'. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Will fix that in the next commit. |
||
|
||
public String getICOREURL() { | ||
return URL_PREFIX + id; | ||
} | ||
} |
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.
Nice find :)