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

Implement #820: Add support for INCLUDES/INCLUDES_ALL/INCLUDES_ANY operators in cypher filters #821

Open
wants to merge 8 commits into
base: master
Choose a base branch
from

Conversation

wbenbihi
Copy link

This pull request adds support for the INCLUDES/INCLUDES_ALL/INCLUDES_ANY operators in cypher filters. It includes changes to the match.py file in the neomodel/sync_ directory. The operators allow for filtering based on array values, such as checking if an array contains a specific value or if it contains all or any of a set of values. This enhancement improves the functionality and flexibility of the cypher filters in the codebase.

These operators are usable both in NodeSet.filter & RelationshipManager.match. It is compatible with neomodel.Q filters for NodeSet.filter method.

Issue #820 describes the feature request.

@wbenbihi
Copy link
Author

Quality Gate Failed Quality Gate failed

Failed conditions 48.9% Duplication on New Code (required ≤ 3%)

See analysis details on SonarCloud

It appears most of the duplicated code comes from the Async to Sync transpiling. Should the _sync updates be commited in PRs or the CI takes care of it?

@mariusconjeaud
Copy link
Collaborator

Hello ! Thank you for your PR.
The way the transpiling works is that you should only write code in the async_ part, and the sync_ part will be generated automatically.

@wbenbihi
Copy link
Author

Hello ! Thank you for your PR. The way the transpiling works is that you should only write code in the async_ part, and the sync_ part will be generated automatically.

Thank you for the clarification. I will revert the code from the sync_ modules

@mariusconjeaud
Copy link
Collaborator

Thank you for the thorough testing !

I just realized I misled you earlier. You should push the sync_ code generated automatically by the pre-commit hooks. I only meant that you should write async_ and let those do the work, instead of the other way around.
And yes, sadly it creates code duplication but that is OK in this context. Maybe I should find a way to have SonarCloud ignore the content of sync_ since it's 100% transpiled.

Copy link

sonarcloud bot commented Jul 30, 2024

@wbenbihi
Copy link
Author

No problem. I just added the sync_ file with make-unasync as described in the contributing guide. The SonarCloud should trigger a Code Duplication warning. I'll let you tell me if the PR is approved.

@mariusconjeaud mariusconjeaud added this to the 5.3.3 milestone Aug 12, 2024
@mariusconjeaud
Copy link
Collaborator

I think I found the way to make Sonarcloud ignore the sync_ folders during the Code Duplication step ! So it's all good now.

@mariusconjeaud
Copy link
Collaborator

Did you read my comments on the PR ?

Also, could you rebase from the rc/5.3.3 branch ? I fixed a bug in a test that was coming from the latest version of Neo4j changing the default ordering - and the test was relying on the exact order without enforcing it...

@mariusconjeaud mariusconjeaud modified the milestones: 5.3.3, 5.4.0 Sep 23, 2024
@mariusconjeaud mariusconjeaud modified the milestones: 5.4.0, 5.4.1 Nov 5, 2024
@mariusconjeaud mariusconjeaud linked an issue Nov 5, 2024 that may be closed by this pull request
@mariusconjeaud mariusconjeaud modified the milestones: 5.4.1, 5.5.0 Nov 22, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

ArrayProperty includes filters operator
2 participants