[GitHub] lucenenet issue #188: Fixed 64 Failing Facet Tests and Finished Facet Implem...

[csharptest]

Github user csharptest commented on the issue:

https://github.com/apache/lucenenet/pull/188
  
No problem, glad you like it. Should serve you well as its been in 
production for years now without issue. Do let me know if you need advice on 
usage. 


-- Roger


> On Oct 3, 2016, at 2:03 PM, Shad Storhaug  
wrote:
> 
> Okay, I added the LurchTable to the Support namespace - turned out to be 
pretty straightforward. This one is now ready to pull (I still can't push...).
> 
> Thanks @csharptest for permission to use your creation. And thanks 
@eladmarg for tracking it down.
> 
> —
> You are receiving this because you were mentioned.
> Reply to this email directly, view it on GitHub, or mute the thread.
> 



---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] lucenenet issue #188: Fixed 64 Failing Facet Tests and Finished Facet Implem...

[NightOwl888]

Github user NightOwl888 commented on the issue:

https://github.com/apache/lucenenet/pull/188
  
@synhershko 

> I say let's first release, then see who uses this. Once we see a lot of 
usage, we may reconsider the implementation. WDYT?

In that case this is ready to go. But if we can spend 2-3 hours copying 
over an existing implementation that is almost a drop-in replacement for what 
we need, why not do one better?

Personally, I have a stake in faceted search - heck I have a whole open 
source project [BoboBrowse.Net](https://github.com/NightOwl888/BoboBrowse.Net), 
and I would like to have alternatives.

Also, Facet and Suggest are both brand new things to Lucene.Net. If you 
want to sway people's attention to this project, give them features they don't 
already have in Lucene.Net 3.0.3 (and make them as good as they can be). I have 
seen a few questions on the user mailing list from people who were trying to 
get this to work. Why not give them something useful?

> Let's have this discussion in the dev@ mailing list please. The list Shad 
provided may be correct, but our priorities are different - the spatial module 
needs work, and there are still failing tests at core. We can also skip 
Analysis.Kuromoji and Analysis.SmartCNand completely now - they aren't worth 
our efforts now. So, Let's have that discussion in the right place so other 
people could chime in as well.

Actually, I am nearly done with Analysis.Stempel now. I know it is not that 
critical, but it looked to be less than a day of work to finish and I started 
it before I knew whether these pull requests were going to be merged.

Also, nearly all of the "failures" in the core are due to the test runners 
screwing up. Many tests are in abstract classes that are meant to be run for 
multiple test subclasses. Those subclasses provide the setup that is necessary 
for the tests to run successfully. But, depending on what test runner you use, 
they are either being run without the setup *and* are being skipped or are 
being run *one additional time* in the abstract class without any setup, 
causing them to fail in the out-of-context run.

I know @conniey is currently working on setting up xUnit and I am hoping 
that its test runner(s) and/or specialized attributes are also a fix for this 
issue. If not, we need to find another solution that works regardless of the 
test runner used. The only thing I have come up with so far is to make those 
tests virtual, override them in the sub classes and only put the `[Test]` 
attribute on the stub methods in the sub classes, not in the abstract class. In 
fact, I did that in the Suggest tests before I realized how widespread this 
issue is (and that it also affects the QueryParser tests). It works, but 
creates a lot of duplicate stub methods that do nothing but call their base 
class.

And yes, I will continue this on the mailing list...

> On that note, @NightOwl888 I have no idea where you are located, but if I 
happen to be in your neighborhood during my travels do let me know and I'll buy 
you beers.

I am in Jomtien Beach, Thailand. And I'll take them draft, thank you :).




---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] lucenenet issue #188: Fixed 64 Failing Facet Tests and Finished Facet Implem...

[synhershko]

Github user synhershko commented on the issue:

https://github.com/apache/lucenenet/pull/188
  
> A plain old generic Dictionary works fine, but may use more RAM than the 
designers intended.

I am totally okay with that. We should aim to get something out the door 
asap, and then optimize later.  This shouldn't keep us from going ahead, and 
I'd consider this a 80/20 case.

I say let's first release, then see who uses this. Once we see a lot of 
usage, we may reconsider the implementation. WDYT?

> if you really want to thank me for doing this, please spend a weekend 
porting one of the remaining sections that doesn't have an open pull request.

Indeed @eladmarg :)

On that note, @NightOwl888 I have no idea where you are located, but if I 
happen to be in your neighborhood during my travels do let me know and I'll buy 
you beers.

> in another subject, which of the sub-project not ported yet?

Let's have this discussion in the dev@ mailing list please. The list Shad 
provided may be correct, but our priorities are different - the spatial module 
needs work, and there are still failing tests at core. We can also skip 
Analysis.Kuromoji and Analysis.SmartCNand completely now - they aren't worth 
our efforts now. So, Let's have that discussion in the right place so other 
people could chime in as well.

And thank you @csharptest :) -- @eladmarg @NightOwl888 let's see if we can 
wrap this PR soon and move on to finish up the rest of the core and more 
important stuff.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] lucenenet issue #188: Fixed 64 Failing Facet Tests and Finished Facet Implem...

[csharptest]

Github user csharptest commented on the issue:

https://github.com/apache/lucenenet/pull/188
  

I have no problem with this. Feel free to extract it. You can probably 
leave the interfaces behind if you don't need them. 


-- Roger
ro...@csharptest.com


> On Oct 2, 2016, at 1:54 AM, eladmarg  wrote:
> 
> there are few files of the LurchTable, so all we need is @csharptest 
permission, and then we can just copy them to the solution and fork the 
relevant parts.
> 
> I think if we'll contact @csharptest, he won't have any opposition.
> The LurchTable is a tested and working solution already, so i think its 
better to use what's working.
> 
> @synhershko is it possible to email @csharptest ?
> 
> —
> You are receiving this because you were mentioned.
> Reply to this email directly, view it on GitHub, or mute the thread.
> 



---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] lucenenet issue #188: Fixed 64 Failing Facet Tests and Finished Facet Implem...

[eladmarg]

Github user eladmarg commented on the issue:

https://github.com/apache/lucenenet/pull/188
  
there are few files of the LurchTable, so all we need is @csharptest 
permission, and then we can just copy them to the solution and fork the 
relevant parts.

I think if we'll contact @csharptest, he won't have any opposition.
The LurchTable is a tested and working solution already, so i think its 
better to use what's working.

@synhershko is it possible to email @csharptest ?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] lucenenet issue #188: Fixed 64 Failing Facet Tests and Finished Facet Implem...

[eladmarg]

Github user eladmarg commented on the issue:

https://github.com/apache/lucenenet/pull/188
  
Hi, 
just found a great implementation which does exactly what is expected 
(truly same as java)

please take a look at : 
[CSharpTest.Net.Collections](https://github.com/csharptest/CSharpTest.Net.Collections),
 there is a [nuget](https://www.nuget.org/packages/CSharpTest.Net.Collections/) 
package as well.

at the constructor use:
`new LurchTable(LurchTableOrder.Access, capacity, 
EqualityComparer.Default)
`
the lucene get method expect for default(TValue) or null, but the 
LurchTable throw exception,
so just wrap the get method with:
` TValue result;
if (!lru.TryGetValue(key, out result))
{
return default(TValue);
}
return result;
`
all tests working, (include the LruHashMap), performance much faster.



---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] lucenenet issue #188: Fixed 64 Failing Facet Tests and Finished Facet Implem...

[NightOwl888]

Github user NightOwl888 commented on the issue:

https://github.com/apache/lucenenet/pull/188
  
> which of the sub-project not ported yet?

AFAIK, these are the pieces that are missing that are not currently being 
worked on, but you should send an email to `dev@lucenenet.apache.org` and ask 
for the all clear to prevent any duplicate work (and if you don't receive a 
reply, consider it a "go ahead").

```
AFAIK, No External Dependencies (may want to double check, though)

Analysis.Kuromoji
Analysis.SmartCN
Analysis.Stempel
Demo
Replicator
QueryParser.Flexible (QueryParser has open pull requests...better to wait 
until they are merged)

With External Dependencies 

Analysis.ICU (Depends on ICU4j)
Analysis.Morfologik (Depends on Morfologik)
Analysis.Phonetic (Depends on Apache Commons)
Analysis.UIMA (Depends on Tagger, uimaj-core, WhiteSpaceTokenizer)
Benchmark (many dependencies)
Highlighter (Depends on Collator (which is still being ported) and 
BreakIterator (which we don't have a good solution for yet))
Sandbox (Depends on Apache Jakarta)

```

The projects with external dependencies need some research to find out if 
there are any good ports of those dependencies already out there. Even if they 
don't have, much of the code doesn't depend on those external dependencies, so 
we can get a head start on it by porting the parts that don't.

It almost goes without saying, we need both the code and the tests ported 
so we can see where the bugs are.

Also, be sure you are looking at the [4.8.0 
tag](https://github.com/apache/lucene-solr/tree/releases/lucene-solr/4.8.0/lucene)
 when porting over the code (its difficult enough to get the pieces to fit 
without having to contend with different versions, too).




---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] lucenenet issue #188: Fixed 64 Failing Facet Tests and Finished Facet Implem...

[eladmarg]

Github user eladmarg commented on the issue:

https://github.com/apache/lucenenet/pull/188
  
by the way, another option is to port the java equivalent 
[LinkedHashMap](http://developer.classpath.org/doc/java/util/LinkedHashMap-source.html)
 derived from HashSet and just implement the Access method on Gets.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] lucenenet issue #188: Fixed 64 Failing Facet Tests and Finished Facet Implem...

[eladmarg]

Github user eladmarg commented on the issue:

https://github.com/apache/lucenenet/pull/188
  
Fantastic! this is awesome, thank you so much!
@synhershko please merge
Again, thanks a lot!


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---