Skip to content

Conversation

@orangejulius
Copy link
Member

@orangejulius orangejulius commented Mar 28, 2025

As reported in #1653, there is an issue when negative sources are specified in combination with negative layers. The end result is that the negative layers are ignored.

This issue was likely introduced in #1604 or #1525, the pull requests where we added the concept of negative sources and layers and subsequently improved them.

The change is split into two commits, a pure refactoring for clarity, and then the fix, which is quite short.

Fixes #1653

This just helps separate the "when" from the "how"
As reported in #1653, there is an
issue when negative sources are specified in combination with negative
layers. The end result is that the negative layers are ignored.

This issue was likely introduced in
#1604 or
#1525, the pull requests where we
added the concept of negative sources and layers and subsequently
improved them.

Fixes #1525
t.deepEqual(clean.layers.sort(), expected_layers, 'layer list is reduced to exclude addresses');
t.end();
});

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm struggling to understand a few things about this unit test:

  • The test is named "exclude addresses when negative layers and sources are specified", is this specific to the address layer?
  • clean_layers filters out venue and sorts the array, then expected_layers does the same thing? Is this maybe to avoid sharing the same pointer as clean_layers?
  • The selected sources are not being asserted.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's right, the entire _address_layer_filter file is centered around an optimization to filter out the address layer when two conditions are met:

  • the input has certain parses, is below a certain number of tokens, etc
  • the layers specified by the user are compatible with filtering out addresses. For example layers=addresses means we can't, but layers=-venue means we can.

@orangejulius orangejulius merged commit 34cf61f into master Apr 4, 2025
6 checks passed
@orangejulius orangejulius deleted the fix-layer-and-source-combination branch April 4, 2025 01:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Layers and sources ignored results in ignored sources only

3 participants