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

dada2 fixes #2705

Merged

Conversation

bernt-matthias
Copy link
Contributor

detected here #2701

  • data manager: real bug fix
  • bug fix in makeSequencTable: cheetah for testing plot variable was wrong
  • allow larger delta for tests on Rdata and pdf (mostly 1/2 file size, but anyway less then the file size .. essentially now a test for file presence)

FOR CONTRIBUTOR:

  • - I have read the CONTRIBUTING.md document and this tool is appropriate for the tools-iuc repo.
  • - License permits unrestricted use (educational + commercial)
  • - This PR adds a new tool or tool collection
  • - This PR updates an existing tool or tool collection
  • - This PR does something else (explain below)

@bgruening
Copy link
Member

@bernt-matthias can you maybe use galaxyproject/galaxy#8852 ... and just check for size. This way we also remove the Rdata from the repo.

@bernt-matthias
Copy link
Contributor Author

Not sure if file size is the same on all systems since the files can be compressed with different methods or even uncompressed.

We could fix to uncompressed. Which would be faster, but require a bit more space. My guess would be that the file size might be the same on all systems.

@bgruening
Copy link
Member

@mvdbeek
Copy link
Member

mvdbeek commented Nov 30, 2019

Oh yeah, it's not in master yet, I'll do that. There's also galaxyproject/galaxy#9054 to look out for.

@mvdbeek
Copy link
Member

mvdbeek commented Nov 30, 2019

master is updated now

@bernt-matthias
Copy link
Contributor Author

@bgruening sounds good. just was wondering if --update_test_data updates the numbers ..? Probably not... :)

@bernt-matthias
Copy link
Contributor Author

bernt-matthias commented Nov 30, 2019

Seems that also galaxyproject/galaxy#8852 has not found its way to master and 19.09 which would be necessary. .. and I think also the xsd of planemo.

@bernt-matthias
Copy link
Contributor Author

Anyway, I just updated the test and do not see the advantage here: We can't remove the test outputs, since subsequent steps of the dada2 pipeline use them as input.

My last commit could be easily undone if you agree.

@bgruening
Copy link
Member

Imho it makes a lot of sense for PDF ... that tools read Rdata is not good, security wise. But I guess this is unrelaed to this PR.

@bernt-matthias
Copy link
Contributor Author

that tools read Rdata is not good, security wise. But I guess this is unrelaed to this PR.

still did not get this point. as far as I see it nothing is executed on import.

detected here galaxyproject#2701

- data manager: real bug fix
- bug fix in makeSequencTable: cheetah for testing `plot` variable was
wrong
- allow larger delta for tests on Rdata and pdf (mostly 1/2 file size,
but anyway less then the file size .. essentially now a test for file
presence)
@bernt-matthias bernt-matthias force-pushed the topic/dada2-container-fixes branch from dd5d47d to ed6d26a Compare December 1, 2019 12:04
@bernt-matthias
Copy link
Contributor Author

@mvdbeek to make this work we need galaxyproject/galaxy#8852 in master and 19.09 .. and I think also the xsd of planemo needs an update

@mvdbeek
Copy link
Member

mvdbeek commented Dec 5, 2019

It isn't a bug, I don't think we can do a backport. But it might not be required if we start using using the proper galaxy packages instead of galaxy-lib in planemo. @nsoranzo was working on this.
Can we just work with the delta for now ?

@bernt-matthias
Copy link
Contributor Author

yep. will reset to 770ea38

@bernt-matthias bernt-matthias force-pushed the topic/dada2-container-fixes branch from ed6d26a to 770ea38 Compare December 5, 2019 10:10
mvdbeek added a commit to mvdbeek/tools-iuc that referenced this pull request Dec 5, 2019
@bgruening bgruening merged commit a82e498 into galaxyproject:master Dec 5, 2019
@bernt-matthias bernt-matthias deleted the topic/dada2-container-fixes branch March 8, 2020 10:37
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.

3 participants