-
Notifications
You must be signed in to change notification settings - Fork 62
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
Create defaultSession spec for config.json files to load simplified default sessions #4148
base: main
Are you sure you want to change the base?
Conversation
A CLI tool could be made to restore the lost functionality from set-default-session --tracks and --view not working, and make it instead set-default-session-spec or something like this |
Note: The defaultSessionSpec takes priority over defaultSession, and defaultSession is not used when defaultSessionSpec is available. If this behavior is confusing, we could also make a "session spec" a special type of defaultSession |
and just for background, the term spec is short for 'specification' in full...just specifies the parameters for the session |
229adc7
to
dbac84d
Compare
in order to avoid confusion between defaultSession and defaultSessionSpec, I propose now making it so it is a special type of defaultSession where defaultSession.type==='spec' |
dbac84d
to
767d8ef
Compare
767d8ef
to
9c093f1
Compare
Hi @cmdcolin, |
this is a good question. one thing i'm debating in this PR is that currently both the "simplified" and "full" snapshot use the "defaultSession" variable name. possibly the simplified one could use a different name. not sure which is better, maybe less ambiguous to separate them the other, sort of more radical way to address this issue, is to actually simplify the internal state of the app to some extent, so that the "full snapshots" provide easy to use mechanisms e.g. specifying a locstring. currently, variables like offsetPx and bpPerPx are used in the snapshots, which is not easy to use programmatically. I would really like to get to the stage that this could be done. if you are interested in trying to find solutions let me know. this i think will probably remain draft for a bit while i ponder it, but you are welcome to try it out ("jbrowse create --branch default_session_spec yournewinstance"). i'd be happy to also advise on other usages like the iframe and embedded too |
Thanks!
ok, both should be ok for us I guess, though a separate variable would probably be less ambiguous/easier to document
sounds like a good plan indeed!
ok nice, for now I think I'll go with an iframe and try to ship this galaxy tool asap, but I'd be very happy to test this new way a bit later! |
Currently, the defaultSession is a raw snapshot of internal app state. There are efforts that can be made to simplify that internal state to some extent, but in addition to or alternative to that, we can create helpers like this PR does that make extra APIs to load data instead of providing raw app state to defaultSession.
This PR is essentially making the "url query param" API available from the config.json on a slot named "defaultSessionSpec", similar to "spec-" type URLs that the jbrowse-web app accepts