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

nb2galaxy prototype #102

Merged
merged 35 commits into from
Feb 27, 2024
Merged

nb2galaxy prototype #102

merged 35 commits into from
Feb 27, 2024

Conversation

dsavchenko
Copy link
Member

@dsavchenko dsavchenko commented Aug 31, 2023

  • add tests
  • add help page
  • add unit tests for nb2galaxy
  • add .shed.yml ?

@codecov-commenter
Copy link

codecov-commenter commented Aug 31, 2023

Codecov Report

Attention: 383 lines in your changes are missing coverage. Please review.

Comparison is base (caef905) 21.49% compared to head (da8eb0f) 18.89%.

Files Patch % Lines
nb2workflow/galaxy.py 0.00% 383 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master     #102      +/-   ##
==========================================
- Coverage   21.49%   18.89%   -2.61%     
==========================================
  Files          32       33       +1     
  Lines        2782     3165     +383     
==========================================
  Hits          598      598              
- Misses       2184     2567     +383     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@volodymyrss
Copy link
Member

volodymyrss commented Sep 5, 2023

try this https://anaconda.org/mmoda/oda-api ? seems to work for me! Here I adapted my trivialized GRB detector, it clearly makes uses oda-api mechanisms for constructing standard LightCurve:

image

I like how this works! We can maybe soon enough integrate it into bot.

There some problems with JSON serialization still I had to make small patches, but it should not be a big problem to fix I imagine, and this adaptation might improve serialization behavior in other cases. Maybe we need to add more tests for this case.

@dsavchenko
Copy link
Member Author

I can't make it work for me. I added mmoda into conda_ensure_channels in galaxy.yml
Then I start galaxy with planemo serve --galaxy_root /path/to/galaxy/installation. But conda still only consult conda-forge, bioconda, default when resolving dependencies, according to the logs.

Did you do it somehow differently?

@volodymyrss
Copy link
Member

I can't make it work for me. I added mmoda into conda_ensure_channels in galaxy.yml Then I start galaxy with planemo serve --galaxy_root /path/to/galaxy/installation. But conda still only consult conda-forge, bioconda, default when resolving dependencies, according to the logs.

Did you do it somehow differently?

I did this:

planemo s --galaxy_root ..../galaxy-fork --conda_channels mmoda,conda-forge

I verified that it uses oda-api from some mulled-... environment.

Maybe your config does not work correctly?

@dsavchenko
Copy link
Member Author

Planemo command line option works, thanks.

Seems that planemo ignores the configuration in galaxy_root.

@volodymyrss volodymyrss self-requested a review September 7, 2023 19:48
@volodymyrss
Copy link
Member

oda-api is in conda-forge now

@bgruening
Copy link

Oh, this is cool. Have you seen this repo? https://github.com/hexylena/galaxyxml/

It should make the construction of the Galaxy tool easier.

Also please have a look at galaxyproject/planemo#1263

@volodymyrss
Copy link
Member

Oh, this is cool. Have you seen this repo? https://github.com/hexylena/galaxyxml/
It should make the construction of the Galaxy tool easier.

Looks interesting, I did not know, could it be useful @dsavchenko , you seem to re-implement part of that ?
I wonder though if galaxyxml will be sustained as part of the Galaxy organization?

Also please have a look at galaxyproject/planemo#1263

Interesting, would it even make sense to have something like planemo tool_init ... --autonbgen ... following papermills-like parameter annotations?

Although, we are also using further conventions about the repository with notebooks, which are not so widespread, so it seems best to keep them out of planemo.

@bgruening
Copy link

bgruening commented Sep 23, 2023

I wonder though if galaxyxml will be sustained as part of the Galaxy organization?

Helena is a long-time contributor, I would not worry about that sustainability here. And yes if needed I'm sure we can move it under the galaxyproject GitHub organisation.

Interesting, would it even make sense to have something like planemo tool_init ... --autonbgen ... following papermills-like parameter annotations?

At some point yes. At least if this can be made general enough. But for the time being, I think it makes more sense to have it outside to iterate faster.

@volodymyrss
Copy link
Member

I wonder though if galaxyxml will be sustained as part of the Galaxy organization?

Helena is a long-time contributor, I would not worry about that sustainability here. And yes if needed I'm sure we can move it under the galaxyproject GitHub organisation.

Interesting, would it even make sense to have something like planemo tool_init ... --autonbgen ... following papermills-like parameter annotations?
At some point yes. At least if this can be made general enough.

I think papermill-like parameter annotations are quite general, aren't they?

But for the time being, I think it makes more sense to have it outside to iterate faster.

Right, makes sense.

@dsavchenko
Copy link
Member Author

Oh, this is cool. Have you seen this repo? https://github.com/hexylena/galaxyxml/

I haven't seen it. It wasn't too complicated to implement what I did just following the galaxy xml doc. But probably it will make sense to migrate some parts in the future. I will take a look, thank you.

Also please have a look at galaxyproject/planemo#1263

This seems to serve for another use-case, the key requirement is that a code uses ArgumentParser. Still, the idea is the same, of course

@volodymyrss
Copy link
Member

Also please have a look at galaxyproject/planemo#1263
This seems to serve for another use-case, the key requirement is that a code uses ArgumentParser. Still, the idea is the same, of course

The idea is analogous, we just have another way to define parameters, with papermill-like annotation. It's not as common as ArgumentParser, but it's also reasonably generic.

@beatrizserrano
Copy link

This would be useful for the imaging community, there are plenty of notebooks available, but not that many Galaxy tools

@volodymyrss
Copy link
Member

You added some gitignore in tests, but did you add tests?

@dsavchenko
Copy link
Member Author

You added some gitignore in tests, but did you add tests?

Not yet

@volodymyrss
Copy link
Member

we'll need an explanation how to make use of this in here oda-hub/hugo-odahub#72

@dsavchenko dsavchenko mentioned this pull request Feb 16, 2024
1 task
@volodymyrss
Copy link
Member

You want to update with base branch?

Copy link
Member

@volodymyrss volodymyrss left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's quite a standalone part and I do not really have any substantial comments.

@volodymyrss
Copy link
Member

Just update the base branch please.

@dsavchenko
Copy link
Member Author

It's quite a standalone part and I do not really have any substantial comments.

Could you merge? I don't have sufficient rights in this repo

@volodymyrss volodymyrss enabled auto-merge (squash) February 26, 2024 20:14
@volodymyrss volodymyrss merged commit 07bdb17 into master Feb 27, 2024
2 checks passed
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.

5 participants