Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Allow evaluation of "top of set" and "bottom of set" ECL expressions #1331

Merged
merged 6 commits into from
Oct 3, 2024

Conversation

apeteri
Copy link
Member

@apeteri apeteri commented Oct 2, 2024

No description provided.

- "top of set" expressions
- "bottom of set" expressions
- alternate identifiers (recognized but currently unsupported)
@apeteri apeteri requested review from cmark and nagyo October 2, 2024 12:14
@apeteri apeteri self-assigned this Oct 2, 2024
Copy link

codecov bot commented Oct 2, 2024

Codecov Report

Attention: Patch coverage is 79.16667% with 10 lines in your changes missing coverage. Please review.

Project coverage is 48.59%. Comparing base (7cca5aa) to head (668c889).
Report is 94 commits behind head on 9.x.

Files with missing lines Patch % Lines
...wl/snomed/core/ecl/SnomedEclEvaluationRequest.java 56.25% 3 Missing and 4 partials ⚠️
...snowowl/core/request/ecl/EclEvaluationRequest.java 90.62% 2 Missing and 1 partial ⚠️
Additional details and impacted files
@@             Coverage Diff              @@
##                9.x    #1331      +/-   ##
============================================
+ Coverage     48.57%   48.59%   +0.01%     
- Complexity    14073    14084      +11     
============================================
  Files          1950     1950              
  Lines         95449    95467      +18     
  Branches      11028    11021       -7     
============================================
+ Hits          46363    46388      +25     
  Misses        46049    46049              
+ Partials       3037     3030       -7     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Member

@cmark cmark left a comment

Choose a reason for hiding this comment

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

🥓

Just a small note: the ECLRewriter logic can also be used to convert some constructs to another ones.


private Set<String> toStringSet(final long[] idArray) {
if (idArray == null || idArray.length < 1) {
return newHashSet();
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 just wondering if this should be mutable. Or would it be possible to use an immutable empty set here?

Copy link
Member

@nagyo nagyo left a comment

Choose a reason for hiding this comment

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

LGTM!

Copy link
Member

@cmark cmark left a comment

Choose a reason for hiding this comment

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

🥓

@cmark cmark merged commit d84c47d into 9.x Oct 3, 2024
1 of 2 checks passed
@cmark cmark deleted the feature/SO-6088-ecl22-evaluation branch October 3, 2024 14:35
# for free to join this conversation on GitHub. Already have an account? # to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants