Skip to content

Payload data setter #892

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

Merged
merged 15 commits into from
Aug 23, 2022
Merged

Payload data setter #892

merged 15 commits into from
Aug 23, 2022

Conversation

hepengfe
Copy link
Collaborator

This PR fixes #890

Description of changes

  • support datapack.set_text("text", -1) which is equivalent to append text payload
  • support datapack.set_text("text") which the text payload index is by default 0. When there is no text payloads in the datapack, it will add a new payload.

Possible influences of this PR.

  • user can set/add data of modality in the DataPack easily

Test Conducted

  • it's backward compatible with past tests

@hepengfe hepengfe self-assigned this Jul 26, 2022
@hepengfe hepengfe added topic: data Issue about data loader modules and data processing related topic:cv labels Jul 26, 2022
@codecov
Copy link

codecov bot commented Jul 26, 2022

Codecov Report

Merging #892 (d48b1a9) into master (d5e69a4) will decrease coverage by 0.00%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master     #892      +/-   ##
==========================================
- Coverage   80.95%   80.95%   -0.01%     
==========================================
  Files         254      254              
  Lines       19569    19566       -3     
==========================================
- Hits        15843    15840       -3     
  Misses       3726     3726              
Impacted Files Coverage Δ
forte/data/data_pack.py 85.90% <ø> (ø)
tests/forte/grid_test.py 100.00% <100.00%> (ø)
tests/forte/image_annotation_test.py 100.00% <100.00%> (ø)

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

Copy link
Member

@hunterhector hunterhector left a comment

Choose a reason for hiding this comment

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

interesting idea, could consider improving the docstring, or maybe re-think the behavior so it won't make people confused.

@hepengfe hepengfe marked this pull request as ready for review August 19, 2022 18:35
@mylibrar mylibrar merged commit 588d6bc into asyml:master Aug 23, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
topic:cv topic: data Issue about data loader modules and data processing related
Projects
None yet
Development

Successfully merging this pull request may close these issues.

DataPack setting for Modality data should support setting data in a new payload
3 participants