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

Proposal for a Function to Generate JSON Schema for Workflow Parameter Files #282

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

suecharo
Copy link

@suecharo suecharo commented Feb 5, 2024

From issue #273

@suecharo
Copy link
Author

suecharo commented Feb 5, 2024

I have not yet been able to address the comment discussed in #273 (comment).

@alexiswl
Copy link
Contributor

Impeccable timing @suecharo! I've also drafted up an implementation of this too that addresses this comment.

It also addresses complex input types, such as the inputs required for this pipeline - https://github.com/umccr/cwl-ica/blob/main/workflows/tso500-ctdna-with-post-processing-pipeline/1.2.0--1.0.0/tso500-ctdna-with-post-processing-pipeline__1.2.0--1.0.0.cwl

Or

https://github.com/umccr/cwl-ica/blob/main/workflows/bclconvert-with-qc-pipeline/4.0.3/bclconvert-with-qc-pipeline__4.0.3.cwl

Mind if I put in a PR into this script?

@alexiswl
Copy link
Contributor

On a different note I've also generated a schema for the workflows themselves! common-workflow-lab/cwl-ts-auto#27

@alexiswl alexiswl mentioned this pull request Feb 15, 2024
2 tasks
@suecharo
Copy link
Author

@alexiswl

Mind if I put in a PR into this script?

Of course, thank you.
I have granted you access to https://github.com/suecharo/cwl-utils. (Is this the correct way to do it?)"

@alexiswl
Copy link
Contributor

I have granted you access to https://github.com/suecharo/cwl-utils. (Is this the correct way to do it?)"

Many correct ways, but yes this one works. I've rebased off your branch onto this PR #288

But can PR this branch onto your branch as well? Rather than PR from main on your remote, I'd recommend making a branch on your repo and putting the PR in from that.

My last step is generating a GH actions test to make sure that I can generate a schema for all of the cwl v1.2 tests and then validate the input jsons in the said test directory against the respective workflow input json schemas.

@suecharo
Copy link
Author

@alexiswl Thank you for your advice.

Rather than PR from main on your remote, I'd recommend making a branch on your repo and putting the PR in from that.

Indeed, you are correct. I should have created a separate branch. I have now created a new branch https://github.com/suecharo/cwl-utils/commits/enhancement/cwl-inputs-schema-gen .

Due to GitHub's constraints, we can't change the source branch in a PR after forking. We'll need to make a new PR instead...

But can PR this branch onto your branch as well?

Absolutely, it is possible. In scenarios where both of us are working on similar matters across our forks, it necessitates sending PRs between our forks, doesn’t it? However, given the circumstances with the main branch as mentioned, should there be a need for further modifications on my end, I will send a PR to your alexiswl:enhancement/cwl-inputs-schema-gen .

Furthermore, feel free to close this PR and shift our discussion to #288 if that is more convenient.

@alexiswl
Copy link
Contributor

alexiswl commented Mar 3, 2024

Hi @suecharo,

Sorry for the delay in the response here.

Yes I think it makes sense to move to #288, I'll 'cherry-picked' your commits in #282 and based my code from there.

Main task I think is to test the json-schema generator against the tests in v1.2

@suecharo
Copy link
Author

suecharo commented Mar 4, 2024

@alexiswl

Thank you.
I have confirmed that commits have been cherry-picked in PR #288 .
So, it's ok to close this PR.

P.S., I'm looking forward to the CWL conference!

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.

None yet

2 participants