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.

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

NER #9

Open
wants to merge 32 commits into
base: main
Choose a base branch
from
Open

NER #9

wants to merge 32 commits into from

Conversation

hrshdhgd
Copy link
Collaborator

@hrshdhgd hrshdhgd commented Jul 15, 2021

  • Added runner into requirements.txt
  • Dynamically creates settings.ini based on provided information (path, ontology etc.)
  • Tests added

Addresses #3

Copy link
Collaborator

@cmungall cmungall left a comment

Choose a reason for hiding this comment

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

add a Makefile step for the code maintainer to update the envo termlist

'iter-mode' : 'collection',
'article-format' : 'txt_tsv',
'export_format': 'tsv',
'termlist_stopwords': os.path.join(path,'stopwords','stopWords.txt'),
Copy link
Collaborator

Choose a reason for hiding this comment

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

see how the mixs json is loaded

The general pattern is

  • allow code user to specify their own paths
  • get the right defaults with zero configuration

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

see how the mixs json is loaded

Could you please guide me to a sample mixs.json file?

Copy link
Collaborator

@cmungall cmungall Jul 16, 2021

Choose a reason for hiding this comment

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

MAIN_MODEL_DIR = os.path.join(os.path.abspath(os.path.dirname(__file__)), 'model')
MAIN_SCHEMA_DIR = os.path.join(MAIN_MODEL_DIR, 'schema')
MIXS_SCHEMA = os.path.join(MAIN_SCHEMA_DIR, 'mixs.json')

requirements.txt Show resolved Hide resolved
@hrshdhgd
Copy link
Collaborator Author

add a Makefile step for the code maintainer to update the envo termlist

So if I get this correct, you want me to add a line in the Makefile to generate envo_termlist.tsv ?

@cmungall
Copy link
Collaborator

So if I get this correct, you want me to add a line in the Makefile to generate envo_termlist.tsv ?

Yes

The guiding principle for all repos is that derived files must have provenance and have transparent portable means to recreate

@cmungall
Copy link
Collaborator

can you also add a text. See the geo example for guidance.

Note that after this PR is merged we will integrate the functionality here:

def perform_text_mining(self, sample: SAMPLE, report: AnnotationReport):

and add a test to test_sample_info

 - description: easy to pass TM text
    sample:
      id: TEST:2
      env_broad_scale: terrestrial biome
      env_local_scale: ...
    must_pass: false
    output:
       env_broad_scale: ENVO:nnn....
       env_local_scale: ...
    expected_failures: {}

and another harder one:

 - description: slightly harder TM test
    sample:
      id: TEST:2
      description: my soil sample
    must_pass: false
    output:
       env_medium_scale: ENVO:<ID FOR SOIL>
    expected_failures: {}

@hrshdhgd
Copy link
Collaborator Author

made a few edits, thoughts @cmungall ?

@hrshdhgd hrshdhgd marked this pull request as ready for review July 28, 2021 23:13
@hrshdhgd hrshdhgd requested a review from cmungall July 28, 2021 23:13
Copy link
Collaborator

@cmungall cmungall left a comment

Choose a reason for hiding this comment

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

need to await runner on pypi

zipp==3.5.0; python_version >= '3.6'
runner@git+https://github.com/monarch-initiative/runner.git
Copy link
Collaborator

Choose a reason for hiding this comment

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

  1. We need a pypi release (for which I need to decide on name)
  2. if Pipfile is SOT then this needs to go in Pipfile

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.

2 participants