Boss_access tool refactor #45
Conversation
erikjohnson24
left a comment
erikjohnson24
left a comment
There was a problem hiding this comment.
I think the intern and numpy versions should be pinned before closing. If possible, the xyz vs zyx order argument (optional, default = "xyz") for the output and input should be implemented, but could be skipped and just noted in the documentation.
saber/boss_access/Dockerfile
Outdated
| FROM python:3.6 | ||
|
|
||
| # Install any needed packages specified in requirements.txt | ||
| RUN pip install numpy |
There was a problem hiding this comment.
should we pin version?
saber/boss_access/Dockerfile
Outdated
| RUN pip install scipy boto3 | ||
|
|
||
| # RUN git clone https://github.com/jhuapl-boss/intern.git && cd intern && git checkout RemoteExtension && git pull && python3 setup.py install --user | ||
| RUN pip install intern |
There was a problem hiding this comment.
should we pin version?
saber/boss_access/boss_access.py
Outdated
| ) | ||
|
|
||
| # Change to X,Y,Z for pipeline | ||
| cutout_data = np.transpose(cutout_data, (2, 1, 0)) |
There was a problem hiding this comment.
do we want to make this a arg for the tool? data_order="xyz" or "zyx" with "xyz" as default? This has come up several time as a bug and we have to write transpose steps into other tools!
There was a problem hiding this comment.
this would only be for boss_pull_cutout right? when a user pushes data back to bossdb should we assume that their data is already in xyz order?
There was a problem hiding this comment.
data in bossdb are stored in ZYX order
saber/boss_access/README.md
Outdated
| ``` | ||
| boss_access.py push (-c CONFIG | -t TOKEN) [-b BUCKET] --coll COLL --exp EXP --chan CHAN --coord COORD --dtype DTYPE --itype ITYPE [--res RES] [--xmin XMIN][--xmax XMAX] [--ymin YMIN] [--ymax YMAX] [--zmin ZMIN] [--zmax ZMAX] [--padding PADDING] -o OUTPUT [--s3-only] | ||
| ```bash | ||
| boss_access.py push (-c CONFIG | -t TOKEN) --coll COLL --exp EXP --chan CHAN [--res RES] [--xmin XMIN][--xmax XMAX] [--ymin YMIN] [--ymax YMAX] [--zmin ZMIN] [--zmax ZMAX] --type TYPE -i INPUT |
There was a problem hiding this comment.
We should add documentation that says what bossdb stack it defaults to. I'm assuming if you provide a token it defaults to 'api.bossdb.io' but maybe if you want to use 'api.theboss.io' you have to provide a config file with the host/protocol you want.
There was a problem hiding this comment.
I'm unfamiliar with api.theboss.io, is there also a public token for that host?
There was a problem hiding this comment.
Yeah so thats the MICrONS stack so basically there are a couple different "bosses" out there. There is the public stack (api.bossdb.io), microns stack (api.theboss.io), test stack (api.bossdbtest.io), and personal dev stacks (api-xenesd1.thebossdev.io). All of them have separate data, user accounts, settings, etc. So to use this tool for another stack there should be two additional inputs:
protocol (optional, str): defaults to 'https'
host (optional, str): defaults to 'api.bossdb.io'
That way for regular use-cases I dont have to worry about remembering the public boss api domain but still have the option to use another stack if needed.
saber/boss_access/boss_access.py
Outdated
| COLL_NAME, | ||
| EXP_NAME, | ||
| type=intern_chans._channel.type, | ||
| datatype=intern_chans.dtype |
There was a problem hiding this comment.
have we tested this code with creating a new channel? I think the array function creates a new channel when it doesnt exist so it might not need the secondary logic
saber/boss_access/boss_access.py
Outdated
| EXP_NAME = args.exp | ||
| CHAN_NAME = args.chan | ||
|
|
||
| if args.type == 'image': |
There was a problem hiding this comment.
I think it would be better if instead we just let user specify dtype explicitly, because that would allow option for uint16 data OR we can just get is straight from the numpy array. So if dtype is specified we use that but if its not specified we just default to input_array.dtype
There was a problem hiding this comment.
yes! we know that uint8 or uint16 is image and uint64 is annotation, so we don't need the user to specify image type. Are there any other image types besides 'image' or 'annotation'?
dxenes1
left a comment
dxenes1
left a comment
There was a problem hiding this comment.
Some small changes for some of the input arguments and I think it's set!
4 participants
Updated boss_access.py and associated Dockerfile and README. The older boss_access.py is still available under boss_access_legacy.py