Skip to content

Biological NER predictor pack() missing context parameter #85

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 28 commits into from
Feb 9, 2022
Merged

Biological NER predictor pack() missing context parameter #85

merged 28 commits into from
Feb 9, 2022

Conversation

hepengfe
Copy link
Collaborator

@hepengfe hepengfe commented Feb 8, 2022

This PR fixes asyml/forte#622.

Description of changes

Add context parameter to the function to adapt API interface changes even though context is not needed.

Possible influences of this PR.

Indexer can run successfully in the clinical pipeline example
https://github.com/asyml/forte/tree/master/examples/clinical_pipeline

Test Conducted

  • provide a sample data that contains biological NER in every data packs and assert the number of NER for each data pack larger than zero
  • check the number of json pickle file generated is same as variable self.num_packs.

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.

And remember to add test cases

from forte.processors.base.batch_processor import RequestPackingProcessor
from ft.onto.base_ontology import EntityMention, Subword
from ft.onto.base_ontology import EntityMention, Subword, Token, Sentence
Copy link
Member

Choose a reason for hiding this comment

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

Should be careful about import orders, this is part of python convention

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Do you mean the order of modules after from ft.onto.base_ontology import should follow alphabetic order?

Copy link
Member

Choose a reason for hiding this comment

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

@hepengfe hepengfe marked this pull request as ready for review February 9, 2022 06:41
@hepengfe hepengfe merged commit 80cfe19 into asyml:main Feb 9, 2022
@hepengfe hepengfe deleted the bio_ner_predictor_fix branch February 9, 2022 21:14
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.

clinical pipeline error
2 participants