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

geo is not searchable #20

Closed
lorenzorietti opened this issue Jul 9, 2016 · 5 comments
Closed

geo is not searchable #20

lorenzorietti opened this issue Jul 9, 2016 · 5 comments

Comments

@lorenzorietti
Copy link

@lorenzorietti lorenzorietti commented Jul 9, 2016

add mapping to gst

@Istar-Eldritch
Copy link
Member

@Istar-Eldritch Istar-Eldritch commented Jul 11, 2016

As I see it we have some options:

  • Have an internal list of the valid db options and check it upfront request
    • This can be created the first time the users uses the tool and use a cached version of it from that moment if hardcoding is not an option.
    • This will also allow the usage of a string metric algorithm for suggestions in case the db is not found.
  • Identify conflictive cases and create the specific mapping for them.
    • This is a simple and fast solution. But won't protect the user from misspells.
  • Do the request but if the following object is returned bail out and notify the user:
{
  "header": {
    "type": "esearch",
    "version": "0.3"
  },
  "esearchresult": {
    "ERROR": "Invalid db name specified: dst"
  }
}
  • The check can be done here bionode-ncbi.js#L523 skipping the retry loop.
  • An internal list of valid dbs can be present for suggestions as in the first case

In my opinion the last option is the most suitable. @bmpvieira If you agree I'll create a pull request to solve it. Do you have any other idea?

@bmpvieira bmpvieira added this to Backlog in Bionode Project Board Mar 29, 2017
@bmpvieira bmpvieira added this to the 1.8.0 milestone Apr 7, 2017
@bmpvieira bmpvieira moved this from Backlog to Next in Bionode Project Board Apr 7, 2017
@bmpvieira
Copy link
Member

@bmpvieira bmpvieira commented Apr 12, 2017

I think we should go with the first option, check if db is valid and if not present list of valid ones (hardcoded in an internal config file in JSON or YAML format).

Here's one retrieved from the select box on NCBI website (not sure if all work with eutils/biondo-ncbi):

"gquery","All Databases"
"assembly","Assembly"
"bioproject","BioProject"
"biosample","BioSample"
"biosystems","BioSystems"
"books","Books"
"clinvar","ClinVar"
"clone","Clone"
"cdd","Conserved Domains"
"gap","dbGaP"
"dbvar","dbVar"
"nucest","EST"
"gene","Gene"
"genome","Genome"
"gds","GEO DataSets"
"geoprofiles","GEO Profiles"
"nucgss","GSS"
"gtr","GTR"
"homologene","HomoloGene"
"medgen","MedGen"
"mesh,"MeSH"
"ncbisearch","NCBI Web Site"
"nlmcatalog","NLM Catalog"
"nuccore","Nucleotide"
"omim","OMIM"
"pmc","PMC"
"popset","PopSet"
"probe","Probe"
"protein","Protein"
"proteinclusters","Protein Clusters"
"pcassay","PubChem BioAssay"
"pccompound","PubChem Compound"
"pcsubstance","PubChem Substance
"pubmed,"PubMed"
"pubmedhealth","PubMed Health"
"snp","SNP"
"sparcle","Sparcle"
"sra","SRA"
"structure","Structure"
"taxonomy","Taxonomy"
"toolkit","ToolKit"
"toolkitall","ToolKitAll"
"toolkitbook","ToolKitBook"
"toolkitbookgh","ToolKitBookgh"
"unigene","UniGene"
@Istar-Eldritch
Copy link
Member

@Istar-Eldritch Istar-Eldritch commented Apr 16, 2017

I'll put some time today into this.

bmpvieira added a commit that referenced this issue Apr 22, 2017
Switch from minimist to yargs for CLI parsing
Add list of possible databases with yargs
Indent JSON output when --pretty
Allow piping JSON, even for fetch
bmpvieira added a commit that referenced this issue Apr 22, 2017
Switch from minimist to yargs for CLI parsing
Add list of possible databases with yargs
Indent JSON output when --pretty
Allow piping JSON, even for fetch
bmpvieira added a commit that referenced this issue Apr 22, 2017
Switch from minimist to yargs for CLI parsing
Add list of possible databases with yargs
Indent JSON output when `--pretty`
Allow piping JSON, even for fetch
@bmpvieira
Copy link
Member

@bmpvieira bmpvieira commented Apr 22, 2017

I've kind of solved this at the CLI level by switching to yargs (#32) but I think it would be nice to also have these kind of checks internally in the library. Maybe we could somehow reuse yargs choice definition.

bmpvieira added a commit that referenced this issue Apr 23, 2017
Better CLI, this fixes #31, #29, #20 and #11
@bmpvieira bmpvieira modified the milestones: 2.1.0, 1.8.0 Apr 25, 2017
@bmpvieira
Copy link
Member

@bmpvieira bmpvieira commented Aug 22, 2017

Fixed by #33. Thanks!

@bmpvieira bmpvieira closed this Aug 22, 2017
@bmpvieira bmpvieira moved this from In Progress to Done in Bionode Project Board Aug 22, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
3 participants