Skip to content

Convert task category to indicate modality#2107

Merged
isaac-chung merged 6 commits into
v2.0.0from
convert-task-category-to-indicate-modality
Feb 20, 2025
Merged

Convert task category to indicate modality#2107
isaac-chung merged 6 commits into
v2.0.0from
convert-task-category-to-indicate-modality

Conversation

@isaac-chung

@isaac-chung isaac-chung commented Feb 20, 2025

Copy link
Copy Markdown
Collaborator

Code Quality

  • Code Formatted: Format the code using make lint to maintain consistent style.

Documentation

  • Updated Documentation: Add or update documentation to reflect the changes introduced in this PR.

Testing

  • New Tests Added: Write tests to cover new functionality. Validate with make test-with-coverage.
  • Tests Passed: Run tests locally using make test or make test-with-coverage to ensure no existing functionality is broken.

@Samoed

Samoed commented Feb 20, 2025

Copy link
Copy Markdown
Member

Can we add new field to TaskMetadata to save information about s2p, p2p and s2s?

@isaac-chung

Copy link
Copy Markdown
Collaborator Author

Can we add new field to TaskMetadata to save information about s2p, p2p and s2s?

A big part of this is accepting that such info can be derived from descriptive stats, as @KennethEnevoldsen elaborated here. The discussion concluded with the proposed changes here.

@Samoed

Samoed commented Feb 20, 2025

Copy link
Copy Markdown
Member

Yes, but can we add it in this PR since we're changing category?

@isaac-chung

Copy link
Copy Markdown
Collaborator Author

This PR is about the breaking change. That's why the target is not main. A warning has been added in the linked PR. Would prefer to keep this unchanged.

@Samoed

Samoed commented Feb 20, 2025

Copy link
Copy Markdown
Member

I see, but we totally don't want to save any information about s2p or p2p?

@KennethEnevoldsen

Copy link
Copy Markdown
Contributor

I see, but we totally don't want to save any information about s2p or p2p?

Wouldn't that just be in the descriptive stats?

@Samoed

Samoed commented Feb 20, 2025

Copy link
Copy Markdown
Member

We have only statistics for this, but no indication for it. We can add property for it

@KennethEnevoldsen

Copy link
Copy Markdown
Contributor

We could do some sort of cutoff or a quick way to access relevant descriptive stats. I could see that it would be nice but I agree with @isaac-chung that it is not for this PR.

@isaac-chung

Copy link
Copy Markdown
Collaborator Author

Hmm the datasets exist but tests fail 🤔

@Samoed

Samoed commented Feb 20, 2025

Copy link
Copy Markdown
Member

Yes, on v2 branch this is every time with the same dataset (even reuploaded #2097)

reference="https://link.springer.com/chapter/10.1007/978-3-319-18117-2_2",
type="Classification",
category="s2s",
category="t2t",

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I'm not sure that classification and clustering tasks should be considered t2t, as they don't fully fit the definition, but this is a minor

@isaac-chung isaac-chung merged commit 056c09a into v2.0.0 Feb 20, 2025
@isaac-chung isaac-chung deleted the convert-task-category-to-indicate-modality branch February 20, 2025 18:01
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.

3 participants