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.

Sign up for GitHub

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

Jump to bottom

Add Sort.relativeComparator for specifying a comparator with compare #25821

Merged
merged 22 commits into from
Sep 3, 2024
Merged

Add Sort.relativeComparator for specifying a comparator with compare #25821

merged 22 commits into from
Sep 3, 2024

Conversation

jabraham17
Copy link
Member

@jabraham17 jabraham17 commented Aug 27, 2024 •
edited
Loading

Adds the relativeComparator interface to the Sort module. This deprecates the old way of defining comparators with compare methods.

Implements part of #25553

Testing

  • built and checked docs
  • paratest with/without comm

[Reviewed by @lydia-duncan]

@jabraham17 jabraham17 mentioned this pull request Aug 28, 2024
Merged
1 task
jabraham17 added a commit that referenced this pull request Aug 28, 2024
@jabraham17
Add docstring and attribute support to interfaces (#25825)
42f8bb0 
Adds compiler support for interfaces to have attributes applied to them
and have docstrings recognized by chpldoc

Resolves #17383

Relies on
chapel-lang/sphinxcontrib-chapeldomain#95

This PR also updates the interface `Allocators.allocator`.
#25821 will update the
existing Sort module allocators. No other allocators are currently shown
in our documentation

While adding tests of `@unstable` with interfaces, I found that some
tests weren't actually testing what we thought they were (i.e.
`test/unstable-keyword/enum_test1.chpl`), so I updated them

Testing
- [x] full paratest with/without comm

[Reviewed by @lydia-duncan and @DanilaFe]
@jabraham17
add the relativeComparator
5b46f42 
Signed-off-by: Jade Abraham <[email protected]>
@jabraham17
add tests of various error messages with the new interface
65bb469 
Signed-off-by: Jade Abraham <[email protected]>
@jabraham17
fix typo
b990b53 
Signed-off-by: Jade Abraham <[email protected]>
@jabraham17
fix spacing
371b607 
Signed-off-by: Jade Abraham <[email protected]>
@jabraham17
fix error handling
7b8d1cb 
Signed-off-by: Jade Abraham <[email protected]>
@jabraham17
add unstable
4187670 
Signed-off-by: Jade Abraham <[email protected]>
@jabraham17
remove debug print
30bb52f 
Signed-off-by: Jade Abraham <[email protected]>
@jabraham17
text test name
9e03001 
Signed-off-by: Jade Abraham <[email protected]>
@jabraham17
update tests
fe1b035 
Signed-off-by: Jade Abraham <[email protected]>
@jabraham17
add todo for later
5e9fdd2 
Signed-off-by: Jade Abraham <[email protected]>
@jabraham17
fix more uses of the old comparators
e5ed4e6 
Signed-off-by: Jade Abraham <[email protected]>
@jabraham17
fix channel
9e98dd9 
Signed-off-by: Jade Abraham <[email protected]>
@jabraham17
fix error messages
358d20f 
Signed-off-by: Jade Abraham <[email protected]>
@jabraham17
fix more tests
a782428 
Signed-off-by: Jade Abraham <[email protected]>
@jabraham17
fix script
cdfe707 
Signed-off-by: Jade Abraham <[email protected]>
@jabraham17 jabraham17 force-pushed the add-relativeComparator branch from 6e8d2cd to cdfe707 Compare August 28, 2024 18:59
@jabraham17
add unstable test
d1d76f8 
Signed-off-by: Jade Abraham <[email protected]>
@jabraham17
expand interface docs
28f3cce 
Signed-off-by: Jade Abraham <[email protected]>
@jabraham17 jabraham17 marked this pull request as ready for review August 28, 2024 21:37
@jabraham17
work around chapel-lang#25838
9445ea2 
Signed-off-by: Jade Abraham <[email protected]>
@jabraham17
fix silly typo
592b845 
Signed-off-by: Jade Abraham <[email protected]>
@jabraham17
fix misspelled filename
af97599 
Signed-off-by: Jade Abraham <[email protected]>
@jabraham17
update good file to reflect improved errors
7323b9c 
Signed-off-by: Jade Abraham <[email protected]>
@lydia-duncan lydia-duncan self-requested a review August 29, 2024 17:30
lydia-duncan
lydia-duncan reviewed Aug 29, 2024
View reviewed changes
Copy link
Member

@lydia-duncan lydia-duncan left a comment

Choose a reason for hiding this comment

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

Thanks for all your hard work here! I had some questions and minor nits. Sorry if any of the questions are things you told me about and I failed to recognize in context.

modules/standard/Sort.chpl Show resolved Hide resolved
modules/standard/Sort.chpl Show resolved Hide resolved
modules/standard/Sort.chpl Outdated Show resolved Hide resolved
modules/standard/Sort.chpl Show resolved Hide resolved
modules/standard/Sort.chpl Outdated Show resolved Hide resolved
test/library/packages/Search/correctness.chpl Show resolved Hide resolved
test/library/standard/Sort/correctness/sort-array-of-owned.chpl Show resolved Hide resolved
test/unstable/sortInterfaces.good Show resolved Hide resolved
@jabraham17
apply feedback
86e5674 
Signed-off-by: Jade Abraham <[email protected]>
@jabraham17 jabraham17 merged commit 074b89e into chapel-lang:main Sep 3, 2024
7 checks passed
@jabraham17 jabraham17 deleted the add-relativeComparator branch September 3, 2024 15:13
ShreyasKhandekar added a commit to ShreyasKhandekar/chapel that referenced this pull request Sep 18, 2024
@ShreyasKhandekar
Update Sort module documentation
d2b2efa 
Update the sort module documentation based on the work done in the sort module stabilization subteam.

On a high level
- Added a short intro with a few examples of using `proc sort`, before this change it was a bit wierd to just see Comparators as the first thing in the sort module docs.
- Edited the comparators section to mention the new prescribed way to create and use custom comparators using the newly created interfaces.
- Went over the rest of the doc to update other things that have changed like argument names

This includes work done in the following PRs:

    chapel-lang#25863
    chapel-lang#25852
    chapel-lang#25821
    chapel-lang#25817
    chapel-lang#25813
    chapel-lang#25807
    chapel-lang#25705
    chapel-lang#25703
    chapel-lang#25699
    chapel-lang#25698
    chapel-lang#25586

Signed-off-by: Shreyas Khandekar <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@lydia-duncan lydia-duncan lydia-duncan approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@jabraham17 @lydia-duncan