Initial scholr.ly integration. #44

Closed
wants to merge 4 commits into
from

Conversation

Projects
None yet
3 participants
Contributor

mhluongo commented Feb 18, 2013

I wanted to make sure everything worked before we hook up to production data.

Contributor

rpicard commented Feb 21, 2013

Hey there! Sorry it took a couple of days to get to you. I thought GitHub was set-up to email me when new pull requests are submitted, but it looks like I was wrong!

Now, in response to your code:

  • What is the overall purpose of this plugin? Just so I can get a better idea of who will be using it, and what it helps them do.
  • It looks like this uses "test data." Is there any reason we aren't using the real data yet?
  • You're using some custom CSS in the abstract template. It's generally bes t to stick with plain text in the abstract. Some light markup (i.e. italics) is warranted in some cases, but generally unnecessary.
Member

majuscule commented Feb 21, 2013

Hi @mhluongo,

I'm Dylan, and we've been talking a bit over email. I was waiting to respond until I had a few more things figured out, but I'm glad @rpicard commented, because it brought us to a discussion where he was able to solve one of the issues I was having (on our end). I've reviewed the code, and everything looks good. I'm working on setting up a development machine where we'll be able to test the deployment and review the result.

Please allow us another few days to make sure everything is right, and I will update here.

Thanks again :-)

Contributor

mhluongo commented Feb 21, 2013

@rpicard @nospampleasemam thanks guys, looking forward to it.

Contributor

mhluongo commented Mar 22, 2013

Is everything square on you guys' end?

Contributor

rpicard commented Mar 29, 2013

Hey @mhluongo. I'm really sorry about the delay! We absolutely should not have kept you waiting like this. Dylan (@nospampleasemam) and I are making it a priority to work out the problems we ran into in getting your plugin ready as soon as we can.

I'll keep you updated!

Contributor

rpicard commented Mar 29, 2013

Okay, we've been reviewing the plugin and we have a couple of fixes we'd like you to make:

  • Remove the HTML / CSS from the abstract.
    We prefer to keep things as text. Sometimes HTML can make its way in there if it's needed, but I don't think that's the case here. This includes the links too, since they can actually mess with the ZCI styles (it becomes a link within a link).
  • Reformat the abstract
    We were thinking that this would be a good format:
Bernd Girod is a researcher interested in model-based video encoding, mpeg-4, video coding and multiframe prediction. Girod has 326 papers with 0 coauthors and 1970 citations.

The things to note here are:

  • We rearranged the last sentence
  • We used his last name as the subject of the last sentence
  • If any number is 1, the word (e.g. coauthors, or papers) should switch to the singular (e.g. coauthor or paper)

We both really like the plugin, and we're excited about getting it live. When do you want to make a full dataset available?

Contributor

mhluongo commented Mar 29, 2013

Hm, in Austin for a conference this weekend- I should be able to take care of those changes and make a full dataset available early next week. Exciting stuff :)

Contributor

rpicard commented Mar 29, 2013

Sounds good. Enjoy the conference!

Contributor

mhluongo commented Apr 3, 2013

I just pushed the requested changes. The only thing I'm not certain about is removing the links from keywords- I understand why you guys need that done, but I wish there were an easy way to include related topics with each author (say by linking to a DDG search, or a Wiki instant answer). Anyway, that might be an interesting thing to work on down the road.

I'll have a proper data URL for you guys by tomorrow at the latest. I want to make sure we expose only the most important / complete author profiles for now to keep relevant, and then expand as we're more confident in our data.

Contributor

rpicard commented Apr 4, 2013

@mhluongo Thanks for updating the data. It looks good now. I'll have some others take a look at it while you get that data ready.

Contributor

mhluongo commented Apr 5, 2013

@rpicard I fixed some things to handle names with non-ASCII characters, and added an initial data URL for ~72k authors that I think will be a good start. I figure that every once in a while the URL can be updates, as we get more data and better disambiguate what we have.

The output.txt is now UTF-8 encoded- hopefully that won't be a problem. LMK if there's anything else I can do.

Contributor

rpicard commented Apr 7, 2013

Right now the category doesn't work because there is a limit to how many items we put in a single category. Since all 140,000+ are in the "researchers" category, we'd need to separate them or forgo the category altogether. If you're interested, I think we could make categories for their areas of interest (e.g. 'Social dependency researchers' for https://robert.duckduckgo.com/?q=Ban+Al-ani).

Contributor

mhluongo commented Apr 7, 2013

I think until we have a better understanding of our data- like
relationships between interests, so we can have more general categories-
we should skip the category. WDYT? I suppose part of that decision depends
on the allowed size of categories. Eg if 10k items per category were
allowed, categories like "AI researchers" might be okay, but if that's
still too large research interest categories might make sense.

Also I noticed a typo on the staging site- "more at scholar.ly" instead of "
scholr.ly"- is that on my end or you guys'?

On Apr 7, 2013 2:54 AM, "Robert Picard" notifications@github.com wrote:

Right now the category doesn't work because there is a limit to how many
items we put in a single category. Since all 140,000+ are in the
"researchers" category, we'd need to separate them or forgo the category
altogether. If you're interested, I think we could make categories for
their areas of interest (e.g. 'Social dependency researchers' for
https://robert.duckduckgo.com/?q=Ban+Al-ani).


Reply to this email directly or view it on GitHubhttps://github.com/duckduckgo/zeroclickinfo-fathead/pull/44#issuecomment-16010626
.

Contributor

rpicard commented Apr 8, 2013

Ten thousand per category is going to be too much. I think it makes sense to skip the category here.

That typo was on our end; I just fixed it. Right now the plugin is going through our brief QA process, but I think I've addressed all of the main issues so, barring some big revelation, it should be ready to launch in the next day or so. :)

Contributor

mhluongo commented Apr 10, 2013

Oh, I also noticed the staging site doesn't have our favicon- will it be
there in production?

Let me know when you guys plan to release this- we'll be posting a bit
about it on our end.
On Apr 8, 2013 6:23 AM, "Robert Picard" notifications@github.com wrote:

Ten thousand per category is going to be too much. I think it makes sense
to skip the category here.

That typo was on our end; I just fixed it. Right now the plugin is going
through our brief QA process, but I think I've addressed all of the main
issues so, barring some big revelation, it should be ready to launch in the
next day or so.


Reply to this email directly or view it on GitHubhttps://github.com/duckduckgo/zeroclickinfo-fathead/pull/44#issuecomment-16042926
.

Contributor

rpicard commented Apr 14, 2013

@mhluongo Hey, just wanted to give you a quick update. We're still working out some kinks on our end with triggers. Unfortunately the favicon won't be there on the main site either because it doesn't appear to be working on the service we use (https://getfavicon.appspot.com/). I've blacklisted it so a blank image doesn't appear in its place.

Contributor

mhluongo commented Apr 18, 2013

I've stopped using a redirect for the favicon and made sure it's served directly by our webserver, instead of S3- hopefully that will help. Let me know if you guys want to spitball about triggers- I'd be happy to do what I can to help.

Contributor

rpicard commented Apr 19, 2013

@mhluongo I'll keep an eye on the service to see if it updates with that change. The trigger problems that are left aren't problems with which triggers we're using, it's just some technical issues with them not working if they are used after the search term, e.g. alex pentland scholr.ly instead of scholr.ly alex pentland where scholr.ly is the trigger. I'm thinking that I'll go ahead and deploy anyways and then look into that problem separately so we don't delay getting this out there any longer!

Contributor

rpicard commented Apr 29, 2013

@mhluongo The plugin has been deployed! 🚀

I was able to fix the triggering issues and it's all live.

I'll merge this pull request into the repo and close it out. Let me know if you have any questions!

Contributor

mhluongo commented Apr 30, 2013

Excellent! Thanks :)

Contributor

mhluongo commented Apr 30, 2013

Obviously this isn't a blocker, but is there anything else I can do to get that favicon working? I've moved it to http://scholr.ly/favicon.ico (no redirects like before) and have a link to it from every page on the site (<link rel="shortcut icon" type="image/x-icon" href="/favicon.ico">). We haven't had problems with any modern browsers, but I still can't seem to get it to show up on the service you guys use..

Contributor

rpicard commented Apr 30, 2013

@mhluongo I am really not sure why it isn't working. I've tried decaching it on their servers (they have a decache URL) but it hasn't helped. We are looking at implementing our own favicon retriever down the line, but right now it isn't a priority, so we'll have to see. I've submitted an issue on their issues tracker: potatolondon/getfavicon#16.

@rpicard rpicard closed this May 19, 2013

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