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

Remove galaxy tool util dependency #2195

Closed
wants to merge 2 commits into from
Closed

Conversation

Midnighter
Copy link
Contributor

@Midnighter Midnighter commented Feb 25, 2023

As suggested by @fabianegli in the discussion on Slack, this PR copies the relevant code into the mulled module. However, there are two caveats:

  1. Copying code like this is nasty since we burden ourselves with ensuring its correctness and maintenance. We will also have to watch for upstream changes. However, I expect that this code is extremely stable since any change would break thousands of existing mulled images. So I think it is safe to make an exception here.
  2. And this may be a killer for these changes: The original source code is released under an Academic Free License version 3.0; I have no idea if that is compatible with the MIT License. I suspect that might rather not be the case.

PR checklist

  • This comment contains a description of changes (with reason)
  • CHANGELOG.md is updated

Copy relevant functions from `galaxy.tool_util.deps.mulled.util` into
the mulled module in order to avoid the dependency on the entire
package.
@codecov
Copy link

codecov bot commented Feb 25, 2023

Codecov Report

Merging #2195 (3affb8a) into dev (7f30502) will decrease coverage by 0.15%.
The diff coverage is 66.10%.

@@            Coverage Diff             @@
##              dev    #2195      +/-   ##
==========================================
- Coverage   71.57%   71.43%   -0.15%     
==========================================
  Files          77       77              
  Lines        8369     8415      +46     
==========================================
+ Hits         5990     6011      +21     
- Misses       2379     2404      +25     
Impacted Files Coverage Δ
nf_core/modules/mulled.py 68.88% <66.10%> (-8.89%) ⬇️
nf_core/modules/test_yml_builder.py 50.66% <0.00%> (-1.34%) ⬇️
nf_core/modules/lint/module_patch.py 58.10% <0.00%> (-0.56%) ⬇️
nf_core/sync.py 74.68% <0.00%> (-0.43%) ⬇️
nf_core/components/list.py 68.42% <0.00%> (-0.42%) ⬇️
nf_core/create.py 64.63% <0.00%> (-0.39%) ⬇️
nf_core/utils.py 82.13% <0.00%> (-0.25%) ⬇️
nf_core/components/create.py 79.43% <0.00%> (-0.10%) ⬇️
nf_core/modules/modules_json.py 80.34% <0.00%> (+0.07%) ⬆️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

Copy link
Contributor

@fabianegli fabianegli left a comment

Choose a reason for hiding this comment

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

In my understanding of what I wrote, I never actually proposed plain copy pasting of the code that is now in this PR and I think this is not warranted.

As for the licensing issue: It would be great to have some input from the maintainers of the original repo if this copy/pasting would be regarded as fair use - and I am not sure how this copied code would need to be marked for copyright purposes and if it would have to propagate to the top level of the repository.

Personally, I think this PR in the current state is creating more and more difficult questions than it answers.

@Midnighter
Copy link
Contributor Author

Closing this one. Superseded by #2199.

@Midnighter Midnighter closed this Mar 12, 2023
@Midnighter Midnighter deleted the remove-galaxy-tool-util branch March 12, 2023 17:20
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