Skip to content
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

Figures matching a "then" query that wraps the end of the dance are displayed in dance order #553

Open
chetgray opened this issue Jan 8, 2019 · 5 comments
Labels

Comments

@chetgray
Copy link
Collaborator

chetgray commented Jan 8, 2019

Example:
Turn on the Figures results column.
Search for dances with a circle then a progression then a circle.
The matching figures for Have I Danced this Before? are rendered as:

circle left 4 places
circle right 3 places
turn alone to face new neighbors ⁋

That dance has the circle right and turn to progress at the end of the B2, and the circle left at the beginning of the A1.

#319 #419

@dcmorse dcmorse added the bug label Jan 9, 2019
@dcmorse
Copy link
Collaborator

dcmorse commented Jan 9, 2019

Good catch! Thank you.

chetgray added a commit to chetgray/contradb that referenced this issue Jan 14, 2019
There's no special rendering to note that the match wraps. Perhaps we
should consider that?

See contradb#553
chetgray added a commit to chetgray/contradb that referenced this issue Feb 4, 2019
There's no special rendering to note that the match wraps. Perhaps we
should consider that?

See contradb#553
chetgray added a commit to chetgray/contradb that referenced this issue Feb 4, 2019
Set uses Hash as storage. Hashes enumerate their values in the order
that the corresponding keys were inserted. The latter is in the
language specification. The former may be an implementation detail
(woof).

Consider reimplementing the DanceDatatable.matching_figures_for_*
methods to return Arrays of SearchMatch when order is important.

See contradb#553
@chetgray
Copy link
Collaborator Author

So I've gotten this working, but it's a little hacky, depending on the implementation of the Set class using an underlying Hash.

I'm exploring using Arrays of SearchMatches in some cases in dance_datatable.rb to alleviate this.

chetgray/contradb@develop...fix-matching-figure-order

@chetgray
Copy link
Collaborator Author

chetgray commented Feb 13, 2019

My fix does not work for all queries.

matching_figures Box the Gnat Contra ["then", ["not", ["figure", "swing"]], ["figure", "box the gnat"]] = #<Set: {|0-1%7|, |6/0%7|}>

Ideally, this would flatten as [|6/0%7|, |0-1%7|], I think?

"and" matches where submatches aren't identical aren't yielded, even if they're abutting. Should that be changed?

matching_figures Box the Gnat Contra ["figure", "box the gnat"] = #<Set: {|0%7|, |1%7|}>
matching_figures Box the Gnat Contra ["figure", "box the gnat"] = #<Set: {|0%7|, |1%7|}>
matching_figures Box the Gnat Contra ["then", ["figure", "box the gnat"], ["figure", "box the gnat"]] = #<Set: {|0-1%7|}>
matching_figures Box the Gnat Contra ["figure", "chain"] = #<Set: {|6%7|}>
matching_figures Box the Gnat Contra ["figure", "box the gnat"] = #<Set: {|0%7|, |1%7|}>
matching_figures Box the Gnat Contra ["then", ["figure", "chain"], ["figure", "box the gnat"]] = #<Set: {|6/0%7|}>
matching_figures Box the Gnat Contra ["and", ["then", ["figure", "box the gnat"], ["figure", "box the gnat"]], ["then", ["figure", "chain"], ["figure", "box the gnat"]]] = #<Set: {}>

@dcmorse
Copy link
Collaborator

dcmorse commented Feb 15, 2019

I'm gonna go over this again, to make sure we understand each other. I think we do!

@chetgray writes

Ideally, this would flatten as [|6/0%7|, |0-1%7|], I think?

You mean it would be best if it flattened to [6,0,1], right? The problem comes when it presents matching figures in a way that leads viewers to believe they're contiguous but that are actually not:

The interface spits out:

neighbors balance & box the gnat
partners balance & swat the flea
ladles chain look for new ⁋

but this isn't any 3 contiguous figures in Box the Gnat Contra!

We'd much rather strategically arrange it to this:

ladles chain look for new ⁋
neighbors balance & box the gnat
partners balance & swat the flea

And the best possible rendering would probably be something like this:

gimp

The two vertical lines show individual search_matches/patches. The horizontal line warns that you're wrapping from the B2 to the A1.

I thought about spending the time to build those extra visualization tools, but I wasn't sure if there'd be demand, or if it'd be intuitive for people.

I'm glad you noticed! I didn't think anyone would. :)


@chetgray writes

"and" matches where submatches aren't identical aren't yielded, even if they're abutting. Should that be changed?

You mean even when they're overlapping, like |0-4%8| and |2-4%8| gives nil when isn't it painfully obvious to return |2-4%8|? I'm totally open to this if someone wants it. I think what was going though my head was "this is an expensive test, and I don't think any users care, so I'm going to err on the side of simplicity".

Or do you really mean 'abuts', as if |0-4%8| and |5-6%8| were to return |0-6%8|? In this I legit think nil is the right response.

@dcmorse
Copy link
Collaborator

dcmorse commented Feb 15, 2019

Other thoughts:

  • whoa, what was I thinking when I named SearchMatch.flatten_set_to_index_a that? It takes an array of SearchMatches, and returned a Set. Thanks for making it return an array. Maybe it should be called flatten_to_a or flatten_to_indexes?
  • I'm happy to merge your current branch - it seems to have advantages and no disadvantages, so why not?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants