better heuristic distinction of .d files #3145

MartinNowak · 2016-08-06T10:29:49Z

properly recongnize dtrace probes
recongnize \ in Makefile paths
recongnize single line file.ext : dep.ext make targets
recognize D module, import, function, and unittest declarations
add more representative D samples

rake benchmark:compare
D changed from 31.2% to 28.1%
DTrace changed from 33.5% to 32.5%
Makefile changed from 35.3% to 39.4%

diff of rake benchmark:generate
https://gist.github.com/MartinNowak/6fba5aed9401dba4d4bfd7969a168c61 (URLs relative to github.com/)

no misclassifications except for 2 /etc/init.d shell scripts

see https://gist.github.com/MartinNowak/fda24fdef64f2dbb05c5a5ceabf22bd3 for the scraper used to get test corpora for D, DTrace, and Makefile with extension:d.

corpus had ~276K lines (~22MiB) from 3K files
had to filter out lots of .d files from the binutils testsuite (apparently references file diff tests)

MartinNowak · 2016-08-06T10:35:15Z

samples/D/arrayops.d

+ * Copyright: Copyright Martin Nowak 2016 -.
+ * License:   $(LINK2 http://www.boost.org/LICENSE_1_0.txt, Boost License 1.0)
+ * Authors:   Martin Nowak
+ */


This file being detected as Makefile brought me here.

wilzbach · 2016-08-11T15:44:34Z

Pinging this as I think the false-positive rate is a lot higher!

e.g.

https://github.com/libmir/mir/pull/281/files (most example scripts in examples/flex_plot)
https://gist.github.com/wilzbach/ed5033e5be08b09dc181972a4f56fb7c
https://gist.github.com/wilzbach/f2e5fc48e76c92fef7eb4f8b55b80e4b
https://gist.github.com/wilzbach/afbc2fffd770bfa98c6d353904df687a
...

and a lot more

larsbrinkhoff · 2016-08-23T06:08:58Z

Thank you, @MartinNowak, this looks good! Could you please help by explaining the reasoning behind the regexp used to match function definitions?

wilzbach · 2016-08-23T06:14:38Z

Thank you, @MartinNowak, this looks good! Could you please help by explaining the reasoning behind the regexp used to match function definitions?

From above:

additionally match import statements, function definitions, and unittests all of those should not match DTrace or Makefiles

larsbrinkhoff · 2016-08-23T06:38:41Z

@wilzbach I'm asking for an explanation of how \w+\s+\w+\s*$.*$(?:$.*$)?\s*{[^}]*} matches D function definitions, and why it doesn't match DTrace files.

wilzbach · 2016-08-24T03:20:23Z

On August 23, 2016 8:38:48 AM GMT+02:00, Lars Brinkhoff notifications@github.com wrote:

@wilzbach I'm asking for an explanation of how
\w+\s+\w+\s*$.*$(?:$.*$)?\s*{[^}]*} matches D function
definitions, and why it doesn't match DTrace files.

Because that's the pattern for a function in D and I highly doubt DTrace has this?
The regex above matches this pattern:

<ReturnType> <FunctionName> ( <FunctionParameters>) {...}

D allows templates, hence the optional look ahead:

<ReturnType> <FunctionName> ( <TemplateParameters>)(<FunctionParameters>){ ... }

Or with a real function:

auto myFun(T)(T paramA)  {
    return paramA + 1;
}

For more information, you may also have a look at the spec: http://dlang.org/spec/function.html

Edit: seems like using code examples from an email reply doesn't work, sorry about that.

larsbrinkhoff · 2016-08-24T07:00:49Z

Thank you @wilzbach!

@arfon This looks fine to me. Possibly verify with a .d corpus?

wilzbach · 2016-08-24T07:31:32Z

@arfon This looks fine to me. Possibly verify with a .d corpus?

Didn't @MartinNowak already add multiple files?

In any case a good repository for idiomatic D is of course it's standard library Phobos (about 200 kLoC iirc) and there are many more here on GitHub (the popularity and presence of D is rising slowly, but steadily)

larsbrinkhoff · 2016-08-24T07:35:52Z

@wilzbach I'm alluding to this phrase from CONTRIBUTING.md:

Test the performance of the Bayesian classifier with a relatively large number (1000s) of sample .yourextension files. (ping @arfon or @bkeepers to help with this) to ensure we're not misclassifying files.

wilzbach · 2016-08-24T07:44:33Z

@wilzbach I'm alluding to this phrase from CONTRIBUTING.md:

Oh I didn't read the guide as I am just following this PR. I could also submit more files as a follow-up to this PR?

arfon · 2016-08-24T15:52:47Z

@arfon This looks fine to me. Possibly verify with a .d corpus?

Sure thing @larsbrinkhoff. Do we have a search query I can use to grab these files? I don't see one in this thread.

wilzbach · 2016-08-24T15:59:41Z

Sure thing @larsbrinkhoff. Do we have a search query I can use to grab these files? I don't see one in this thread.

I am pretty sure you are more familiar with your search query, but I would start by fetching some of the most starred repos with language D that have been updated within this year and filter for all files with the suffix ".d"

https://github.com/search?utf8=%E2%9C%93&q=language%3Ad+pushed%3A%3E2016-01-01&type=Repositories&ref=searchresults

arfon · 2016-08-24T16:05:09Z

Thanks @wilzbach. I ended up going with this search string to return file results (we need to use the code search rather than the repository search here). @larsbrinkhoff - I just sent you a corpus - would you be able to take a look?

wilzbach · 2016-08-24T16:15:10Z

Thanks @wilzbach. I ended up going with this search string to return file results (we need to use the code search rather than the repository search here). @larsbrinkhoff - I just sent you a corpus - would you be able to take a look?

Oh, one thing that you might be careful with is that even though D1 has been officially deprecated in 2010, there's still some code using the old language grammar and D2 constantly keeps improving, so I don't know how sensitive your classifier is, but you might want to use a minimum date.

Secondly I noticed that your recognition of language:d isn't perfect, for example the first four files are not D source files:
https://github.com/sjnewbury/multilib-overlay/blob/80c9fd20cfd05481ac19edcadd56ad5e578a8930/net-wireless/bluez/files/4.18/bluetooth-init.d, but I don't how to fix this :/

larsbrinkhoff · 2016-08-25T05:00:23Z

@larsbrinkhoff - I just sent you a corpus - would you be able to take a look?

Thanks. It may take a while before I find a time slot for this.

If @MartinNowak or @wilzbach (or anyone else) feel this is urgent, I can provide instructions for how to evalutate the corpous.

wilzbach · 2016-09-02T06:19:32Z

If @MartinNowak or @wilzbach (or anyone else) feel this is urgent, I can provide instructions for how to evalutate the corpous

Yes I consider this as urgent - how can I help? :)

larsbrinkhoff · 2016-09-02T06:57:02Z

@wilzbach You'll have to install Linguist locally as per the instructions here. Then I can send you a zip file with lots of .d files, and you can run linguist on them to ensure that this PR does the right thing.

wilzbach · 2016-09-02T07:02:10Z

@wilzbach You'll have to install Linguist locally as per the instructions here. Then I can send you a zip file with lots of .d files, and you can run linguist on them to ensure that this PR does the right thing

Sure, that's sounds easy (currently on my commute & can do this tonight) - did you see my comment about the search query finding also non D language files?

larsbrinkhoff · 2016-09-02T07:04:58Z

@wilzbach Great, can I send the file to you gmail address?

Right, language:d isn't perfect. The improved heuristic should make it better.

wilzbach · 2016-09-02T07:06:51Z

@wilzbach Great, can I send the file to you gmail address?

Yes, please do :)

larsbrinkhoff · 2016-09-02T07:08:41Z

You may get some files that are not D, but that's not a bad thing.

MartinNowak · 2016-09-23T15:51:27Z

Could you please send me corpora for D, DTrace, and Makefile @arfon @larsbrinkhoff?

I generated some myself (get corpora for different languages), but using repositories isn't the best source and the code search itself isn't fully available via API (scoped on user/repo).

BTW, there seems to be a fourth contender for the .d extension, apparently it's used by a few people as a game scripting language (currently classified as DTrace), but I don't plan to mess w/ that.

arfon · 2016-09-23T23:29:29Z

Sure thing @MartinNowak:

D
DTrace
Makefile

MartinNowak · 2016-09-26T21:52:05Z

Thanks @arfon, unfortunately the zip files are empty.

arfon · 2016-09-26T21:53:05Z

Thanks @arfon, unfortunately the zip files are empty.

Oh wow. Sorry about that @MartinNowak. Totally should have checked...

MartinNowak · 2016-10-04T20:25:15Z

Did you have time to refetch them @arfon. It seems the rules in this PR could use a bit more tweaking.

arfon · 2016-10-04T20:27:08Z

Did you have time to refetch them @arfon. It seems the rules in this PR could use a bit more tweaking.

I did not I'm afraid. It's slightly more complex than I first though to fix this. We've been re-working how we run all of our asynchronous jobs in our platform and currently the admin tool I have to run these jobs is broken 😞. I'll report back here once this is fixed and we can pull the sample files again.

MartinNowak · 2016-10-17T13:42:05Z

Any other idea? I can try to make my script work a bit better, but using whole repos suffers badly from misclassification.

MartinNowak · 2016-11-10T23:16:11Z

Any workarounds @arfon?

arfon · 2016-11-11T00:15:45Z

Any workarounds @arfon?

Not really sorry and I'm afraid since this Pull Request was opened I've left GitHub so we'll need to get another Linguist maintainer who's on GitHub staff to assist here.

@MikeMcQuaid @lildude @brandonblack - there's an area of staff tools devtools/samples I think that allows you to run a search query and it batches up the first ~1000 files. These are useful for testing new heuristics. Unfortunately the jobs don't seem to be running any longer and I didn't manage to figure out what was going on before I left. Any chance one of you could take a look?

wilzbach · 2016-11-22T19:40:14Z

Not really sorry and I'm afraid since this Pull Request was opened I've left GitHub so we'll need to get another Linguist maintainer who's on GitHub staff to assist here.
@MikeMcQuaid @lildude @brandonblack - there's an area of staff tools devtools/samples I think that allows you to run a search query and it batches up the first ~1000 files. These are useful for testing new heuristics. Unfortunately the jobs don't seem to be running any longer and I didn't manage to figure out what was going on before I left. Any chance one of you could take a look?

Ping @MikeMcQuaid @lildude @brandonblack - this PR is pending for quite a while now. Would you be so kind to find a bit of time to look at the search query? AFAICT we only need to testing this PR against your GitHub corpus.

wilzbach · 2016-12-11T20:51:27Z

Ping @MikeMcQuaid @lildude @brandonblack - this PR is pending for quite a while now. Would you be so kind to find a bit of time to look at the search query? AFAICT we only need to testing this PR against your GitHub corpus.

Friendly re-ping ;-)

Alhadis · 2016-12-12T02:03:28Z

Polite reminder that we all have lives, and Mr McQuaid is responsible for maintaining the biggest (and also the best) package manager in the solar system.

I might look into this later, but pinging my attention will yield the opposite effect.

lildude · 2016-12-12T08:19:04Z

Sorry about the delay here. I started looking into the failing job off your last ping @wilzbach but then my own $job stole my attention. I'll try find some time this week to see if I work out why the job is failing. I have a suspicion I know what the problem is.

- require json for Hash.to_json

- properly recongnize dtrace probes - recongnize \ in Makefile paths - recongnize single line `file.ext : dep.ext` make targets - recognize D module, import, function, and unittest declarations - add more representative D samples D changed from 31.2% to 28.1% DTrace changed from 33.5% to 32.5% Makefile changed from 35.3% to 39.4% See https://gist.github.com/MartinNowak/fda24fdef64f2dbb05c5a5ceabf22bd3 for the scraper used to get a test corpus.

MartinNowak · 2017-01-15T23:21:47Z

Build a scraper myself and improved all the existing heuristics for the .d file ending.

MartinNowak · 2017-01-25T13:23:39Z

Can we get a fresh review on this @Alhadis, @brandonblack?

MartinNowak · 2017-03-14T13:13:42Z

lib/linguist/heuristics.rb

+      # target : \
+      #  : dependency
+      # path/file.ext1 : some/path/../file.ext2
+      elsif /([\/\\].*:\s+.*\s\\$|: \\$|^ : |^[\w\s\/\\.]+\w+\.\w+\s*:\s+[\w\s\/\\.]+\w+\.\w+)/.match(data)


The last regex part for path/file.ext1 : some/path/../file.ext2 is specific to match only files with extensions b/c Symbol : Base is commonly used in the other languages as well.

MartinNowak · 2017-03-14T13:13:46Z

Already rotting since 2 month.
Improves 3 languages, thoroughly tested, documented regexes, shouldn't require much effort to review.
@pchaigno, @brandonblack

Alhadis

Seems pretty solid. I don't know D, but I can't see this being anything short of an improvement.

I meant to get back to this earlier, but depression + busy life, etc... sorry.

larsbrinkhoff · 2017-03-14T14:14:23Z

I'm still in favour of this change. Good work.

Patience and the occasional ping may be required.

MartinNowak · 2017-03-25T16:21:00Z

I meant to get back to this earlier ... sorry.

No worries, fairly busy myself :).

lildude

Overall this PR looks good, just a little question about what appears to be an unnecessary addition.

lildude · 2017-03-26T11:04:17Z

Rakefile

@@ -4,6 +4,7 @@ require 'rake/testtask'
 require 'yaml'
 require 'yajl'
 require 'open-uri'
+require 'json'


I can't see any reason for this addition. Is there a reason for this? If not, can you please remove it?

The rake benchmark task fails w/o it, b/c it uses Hash.to_json. Apparently I was the first to use that task in a while. Also see the separate commit message 7980c3d.
I could for sure separate that out into a separate PR if wanted, but it seems like a tiny fix on-the-go.
@lildude

I could for sure separate that out into a separate PR if wanted, but it seems like a tiny fix on-the-go.

Nah, that's good enough explanation for me to do it here 😄.

MartinNowak · 2017-03-30T18:47:46Z

We made it 🏁!
You guys should add reactions to commits and merge messages 😉.

* github/master: Fix heuristic for Unix Assembly with .ms extension (github-linguist#3550) better heuristic distinction of .d files (github-linguist#3145)

MartinNowak force-pushed the d_heuristics branch from 03d0c9e to 4e336c0 Compare August 6, 2016 10:33

MartinNowak reviewed Aug 6, 2016
View reviewed changes

MartinNowak mentioned this pull request Aug 6, 2016

fix alignment of authors section dlang/druntime#1622

Merged

wilzbach mentioned this pull request Dec 18, 2016

Add simple test suite dlang/dlang-bot#16

Merged

MartinNowak added 2 commits January 15, 2017 22:44

fix benchmark

7980c3d

- require json for Hash.to_json

MartinNowak force-pushed the d_heuristics branch from 4e336c0 to fe069d8 Compare January 15, 2017 23:01

MartinNowak changed the title ~~better heuristic detection of D Programming Language~~ better heuristic distinction of .d files Jan 15, 2017

brndnblck added the Improvement label Feb 22, 2017

MartinNowak commented Mar 14, 2017

View reviewed changes

Alhadis approved these changes Mar 14, 2017

View reviewed changes

larsbrinkhoff approved these changes Mar 14, 2017

View reviewed changes

lildude requested changes Mar 26, 2017

View reviewed changes

lildude approved these changes Mar 30, 2017

View reviewed changes

lildude merged commit fa6ae11 into github-linguist:master Mar 30, 2017

MartinNowak deleted the d_heuristics branch March 30, 2017 18:38

pchaigno mentioned this pull request Aug 24, 2017

Github thinks this file is Makefile instead of D #3462

Closed

github-linguist locked as resolved and limited conversation to collaborators Jun 17, 2024

better heuristic distinction of .d files #3145

better heuristic distinction of .d files #3145

Conversation

MartinNowak commented Aug 6, 2016 • edited Loading

MartinNowak Aug 6, 2016

Choose a reason for hiding this comment

wilzbach commented Aug 11, 2016 • edited Loading

larsbrinkhoff commented Aug 23, 2016 • edited Loading

wilzbach commented Aug 23, 2016

larsbrinkhoff commented Aug 23, 2016

wilzbach commented Aug 24, 2016 • edited by pchaigno Loading

larsbrinkhoff commented Aug 24, 2016

wilzbach commented Aug 24, 2016

larsbrinkhoff commented Aug 24, 2016

wilzbach commented Aug 24, 2016

arfon commented Aug 24, 2016

wilzbach commented Aug 24, 2016 • edited Loading

arfon commented Aug 24, 2016

wilzbach commented Aug 24, 2016

larsbrinkhoff commented Aug 25, 2016

wilzbach commented Sep 2, 2016

larsbrinkhoff commented Sep 2, 2016

wilzbach commented Sep 2, 2016 • edited Loading

larsbrinkhoff commented Sep 2, 2016

wilzbach commented Sep 2, 2016

larsbrinkhoff commented Sep 2, 2016

MartinNowak commented Sep 23, 2016

arfon commented Sep 23, 2016

MartinNowak commented Sep 26, 2016

arfon commented Sep 26, 2016

MartinNowak commented Oct 4, 2016

arfon commented Oct 4, 2016

MartinNowak commented Oct 17, 2016

MartinNowak commented Nov 10, 2016

arfon commented Nov 11, 2016

wilzbach commented Nov 22, 2016

wilzbach commented Dec 11, 2016

Alhadis commented Dec 12, 2016 • edited Loading

lildude commented Dec 12, 2016 • edited Loading

MartinNowak commented Jan 15, 2017

MartinNowak commented Jan 25, 2017

MartinNowak Mar 14, 2017

Choose a reason for hiding this comment

MartinNowak commented Mar 14, 2017

Alhadis left a comment

Choose a reason for hiding this comment

larsbrinkhoff commented Mar 14, 2017

MartinNowak commented Mar 25, 2017

lildude left a comment

Choose a reason for hiding this comment

lildude Mar 26, 2017

Choose a reason for hiding this comment

MartinNowak Mar 30, 2017 • edited Loading

Choose a reason for hiding this comment

lildude Mar 30, 2017

Choose a reason for hiding this comment

MartinNowak commented Mar 30, 2017

MartinNowak commented Aug 6, 2016 •

edited

Loading

wilzbach commented Aug 11, 2016 •

edited

Loading

larsbrinkhoff commented Aug 23, 2016 •

edited

Loading

wilzbach commented Aug 24, 2016 •

edited by pchaigno

Loading

wilzbach commented Aug 24, 2016 •

edited

Loading

wilzbach commented Sep 2, 2016 •

edited

Loading

Alhadis commented Dec 12, 2016 •

edited

Loading

lildude commented Dec 12, 2016 •

edited

Loading

MartinNowak Mar 30, 2017 •

edited

Loading