refgenie / refgenconf

A Python object for standardized reference genome assets.
http://refgenie.databio.org
BSD 2-Clause "Simplified" License
3 stars 6 forks source link

bug in get_remote_data_str ? #96

Closed nsheff closed 4 years ago

nsheff commented 4 years ago

raised by @natefoo, confirmed by me.

it appears you can't retrieve the remote data correctly as it seems to expect only 1 server.

with:

genome_servers: ['http://refgenomes.databio.org', 'http://rg.databio.org:82']

import refgenconf
rgc = refgenconf.RefGenConf("/home/nsheff/Dropbox/env/refgenie_config/zither.yaml")
rgc.get_remote_data_str()

Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/home/nsheff/.local/lib/python3.7/site-packages/refgenconf/refgenconf.py", line 415, in get_remote_data_str
    url = get_url(self[CFG_SERVERS_KEY], API_ID_ASSETS)
  File "/home/nsheff/.local/lib/python3.7/site-packages/refgenconf/refgenconf.py", line 404, in <lambda>
    def get_remote_data_str(self, genome=None, order=None, get_url=lambda server, id: construct_request_url(server, id)):
  File "/home/nsheff/.local/lib/python3.7/site-packages/refgenconf/refgenconf.py", line 1589, in construct_request_url
    return server_url + _get_server_endpoints_mapping(server_url)[operation_id]
  File "/home/nsheff/.local/lib/python3.7/site-packages/refgenconf/refgenconf.py", line 1603, in _get_server_endpoints_mapping
    json = _download_json(url + "/openapi.json")
TypeError: can only concatenate list (not "str") to list
vreuter commented 4 years ago

This is annoying and comes up often enough that it's worth considering a ubiquerg function to handle this, a wrapper that would lift a single string into a list and then concatenate. Could even have a parameter w/ default for what individual types to lift into list. I even thought there was something like this in ubiquerg already, but it must've been either a little helper within another project, or else I'd just thought about it but never added it.

stolarczyk commented 4 years ago

I've implemented such a function in refgenconf:

https://github.com/refgenie/refgenconf/blob/a186c54279eb95ea45626c8ae077bcb764b7ee9c/refgenconf/refgenconf.py#L1496-L1515

nsheff commented 4 years ago

I love the docstring on that function:

Convert a str to list of str or ensure a list is a list of str

stolarczyk commented 4 years ago

@natefoo as per https://github.com/refgenie/refgenconf/issues/86, get_remote_data_str should not be used anymore. The official method for querying remote assets, that supports multiple servers, is listr.

I should have deprecated get_remote_data_str, but evidently I have not, my bad.

natefoo commented 4 years ago

@stolarczyk thanks! It may be worth mentioning the deprecation in the docstring so it ends up in the documentation? That's how I found it in the first place.

stolarczyk commented 4 years ago

you're right, thanks :)