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

rename .name to .element_identifier #239

Merged

Conversation

bernt-matthias
Copy link
Contributor

for the remaining tools

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)

for the remaining tools
@bernt-matthias bernt-matthias force-pushed the topic/name-id branch 3 times, most recently from 0249842 to a7a2f20 Compare May 17, 2023 14:54
@bgruening
Copy link
Contributor

Remove the r-linter ;)

@bernt-matthias
Copy link
Contributor Author

Remove the r-linter ;)

We have to restist this impulse :) linters are less annoying than ugly inconsistent code.

@bernt-matthias bernt-matthias mentioned this pull request May 25, 2023
5 tasks
@lecorguille
Copy link
Member

Hey, ipo was integreted before the arrival of the r-linter 😇

@lecorguille
Copy link
Member

If I merge now, do you know if the "Check file sizes" will be an issue for the publishing in the TS?

@bernt-matthias
Copy link
Contributor Author

If I merge now, do you know if the "Check file sizes" will be an issue for the publishing in the TS?

Deployment is done independent of the "Check file sizes test": https://github.com/workflow4metabolomics/tools-metabolomics/blob/7fa454b6a4268b89fe18043e8dd10f30a7b4c7ca/.github/workflows/pr.yaml#L221C56-L221C56

At IUC we also deploy now also in case of tool linter warnings (but they are still shown in the PRs).

I just reopened because I had the impression that tool tests failed.

@bgruening
Copy link
Contributor

Ship it? Or not?

@lecorguille lecorguille merged commit e963da1 into workflow4metabolomics:master Sep 7, 2023
@bernt-matthias bernt-matthias deleted the topic/name-id branch September 11, 2023 08:15
@bernt-matthias
Copy link
Contributor Author

Seems that this was not deployed: https://github.com/workflow4metabolomics/tools-metabolomics/actions/runs/6115157809

ipo4xcmsSet fails with

Error in ipo4xcmsSet(directory, "IPO_parameters4xcmsSet.tsv", args, samplebyclass) : 
  object 'peakpicking_best_params' not found
Execution halted

There is also a warning, but likely unrelated

rror in plot.new() : could not open file '../IPO_results/rsm_1.jpg'
In addition: Warning message:
In dir.create(subdir) :
  cannot create dir '../IPO_results', reason 'Read-only file system'
Error in plotting (see above), but IPO continues to work normally!

bernt-matthias added a commit to bernt-matthias/xcms that referenced this pull request Sep 11, 2023
workflow4metabolomics#239
has not been deployed because of this bug
@bernt-matthias bernt-matthias mentioned this pull request Sep 11, 2023
5 tasks
bernt-matthias added a commit to bernt-matthias/xcms that referenced this pull request Sep 11, 2023
to deploy the camera tools which did not happen here
workflow4metabolomics#239
bernt-matthias added a commit to bernt-matthias/xcms that referenced this pull request Sep 11, 2023
to trigger deployment which did not happen here
workflow4metabolomics#239
This was referenced Sep 11, 2023
@bernt-matthias
Copy link
Contributor Author

I opened three new PR. The first should fix the IPO bug. The other two just add a bugus change to trigger deployment.

Guess having independent PRs makes deployment easier.

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