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

Use reference-spec-reader pkg for ReferenceStore #83

Merged
merged 3 commits into from
Mar 29, 2021
Merged

Conversation

manzt
Copy link
Member

@manzt manzt commented Mar 28, 2021

No description provided.

@manzt manzt merged commit 8a75590 into master Mar 29, 2021
@manzt manzt deleted the reference-store-pkg branch March 29, 2021 15:38
@oeway
Copy link
Contributor

oeway commented Mar 30, 2021

Hi @manzt This is awesome! Thank you.

For the URL, I have two question regarding this:

  • the reference maker allows specifying the URL as null and then we can define a common URL, can we support this? Maybe allow passing something like reference_spec_url in the URL? This will be desired because for URLs generated with S3, it carries a super long signature
  • not all the url will end with .json, would be nice to have a way to explicitly tell the viewer to parse an arbitrary URL as a reference spec.

@manzt
Copy link
Member Author

manzt commented Mar 30, 2021

Hey! I have a couple of ideas here. My first thought is to adopt something similar to fsspec and and force reference spec urls to have reference:// protocol signify the URL is for a reference, then we can pass other query parameters for the reference url (like target).

e.g. http://hms-dbmi.github.io/vizarr?source=reference://<reference_url>?target=<target_url>

This avoids needing to expose another top-level API input, and instead we can check for the reference:// and parse the complete referenced (for a default target etc).

@manzt
Copy link
Member Author

manzt commented Mar 30, 2021

Ah, but then the reference url might break. I think avoiding special structure of URLs is desirable (moving away from checking for file extensions (e.g. .json or .zarr) and instead being explicit by prepending an additional protocol: reference://.

I still don't know what the best solution for encoding the default target URL, however. I don't totally love the option of adding reference_spec_url because it feels like that information should be provided with the reference source.

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