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

set-default-session does not work since 2.10.2 #4434

Closed
heavywatal opened this issue Jun 13, 2024 · 4 comments · Fixed by #4447
Closed

set-default-session does not work since 2.10.2 #4434

heavywatal opened this issue Jun 13, 2024 · 4 comments · Fixed by #4447
Labels
bug Something isn't working

Comments

@heavywatal
Copy link
Contributor

Describe the bug

I have been using jbrowse set-default-session with --view and --tracks options. Unfortunately they were removed in 2.10.2. The removal was announced as a bug fix, but actually that was a breaking change in the existing API, which I think deserves at least a minor version bump. Anyway, now I am struggling to build a new pipeline to setup a new session via CLI in newer versions. The first attempt was to simply remove --view and --tracks options from the command, but the command resulted in an error "Error: Please either provide a default session file". It does not make sense.

To Reproduce

  1. Create a new config: jbrowse create ...
  2. Add an assembly and tracks to the target config: jbrowse add-assembly ...; jbrowse add-track ...
  3. Try to create a default session to the target config: jbrowse set-default-session ...

Expected behavior

A default session can be created with a view and tracks. Even though the previous behavior of set-default-session was not perfect as stated in #2708 and #4146, it was much better than "not provided at all".

Screenshots

Version:

  • @jbrowse/cli 2.10.1: OK
  • @jbrowse/cli 2.10.2: NG
  • @jbrowse/cli 2.11.2: NG

Additional context

@heavywatal heavywatal added the bug Something isn't working label Jun 13, 2024
@cmdcolin
Copy link
Collaborator

thanks for alerting me to this. I actually didn't realize it was working in any state. we can try to restore it back to where it was at least

@heavywatal
Copy link
Contributor Author

Thank you. But I understand that you don't want to revert and keep incomplete functionalities. So I would suggest not to revert the code, but just to amend the help doc and error messages to guide users properly.

For example, the error message "Error: Please either provide a default session file" was not clear:

  • "either"? or what?
  • is [-s <value>] now mandatory? If so, it should not be an optional argument, but should be a required argument (but making it a positional argument breaks compatibility). The leading title of the help "Set a default session with views and tracks" may be also misleading. Maybe "Set a default session from an existing session file"?

If I understand it correctly, I think I can handle it by creating a session.json from scratch and reading it with -s.

These two lines in EXAMPLES section do not work, I guess:

  • jbrowse set-default-session --target /path/to/jb2/installation/config.json
  • jbrowse set-default-session --view LinearGenomeView, --name newName

@cmdcolin
Copy link
Collaborator

@heavywatal I went ahead and made your suggested documentation fixes!

I think a longer term effort will be to make it so that "defaultSession" format is easier to use, so that it can be hand edited or programmatically generated without as much effort.

There are a couple PR that are open for this but it is a tricky one, so will take some time. Thanks for reporting this though

The related PRs...

#4444
#4148

@heavywatal
Copy link
Contributor Author

Confirmed 2.12.0 works as expected. I am using hand-made session.jsons to feed set-default-session -s. Thank you for the quick action.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants