Skip to content

Regenerate parsers with schema graph property #145

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

Merged
merged 6 commits into from
Aug 25, 2022
Merged

Conversation

GlassOfWhiskey
Copy link
Collaborator

This PR regenerates parser to apply updates introduced in this schema_salad PR.

@GlassOfWhiskey GlassOfWhiskey force-pushed the schema-salad-pr-583 branch 2 times, most recently from 57c8b03 to ccda963 Compare August 21, 2022 12:36
Copy link
Member

@mr-c mr-c left a comment

Choose a reason for hiding this comment

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

Can you modify this to temporarily install schema salad from that other PR so that the import works correctly?

@codecov
Copy link

codecov bot commented Aug 24, 2022

Codecov Report

Merging #145 (59e73fc) into main (9ab5512) will increase coverage by 1.15%.
The diff coverage is 71.71%.

@@            Coverage Diff             @@
##             main     #145      +/-   ##
==========================================
+ Coverage   22.42%   23.58%   +1.15%     
==========================================
  Files          27       27              
  Lines       20629    20719      +90     
  Branches     5762     5777      +15     
==========================================
+ Hits         4627     4886     +259     
+ Misses      14998    14794     -204     
- Partials     1004     1039      +35     
Impacted Files Coverage Δ
cwl_utils/parser/cwl_v1_0.py 27.07% <69.69%> (+0.78%) ⬆️
cwl_utils/parser/cwl_v1_1.py 23.69% <69.69%> (+2.59%) ⬆️
cwl_utils/parser/cwl_v1_2.py 24.89% <75.75%> (+0.89%) ⬆️
cwl_utils/parser/__init__.py 71.13% <0.00%> (+2.06%) ⬆️

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

Copy link
Member

@mr-c mr-c left a comment

Choose a reason for hiding this comment

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

Looking good! Can we get some tests showing off the new functionality?

@GlassOfWhiskey GlassOfWhiskey requested a review from mr-c August 24, 2022 09:45
@GlassOfWhiskey
Copy link
Collaborator Author

GlassOfWhiskey commented Aug 24, 2022

Looking good! Can we get some tests showing off the new functionality?

Some unit tests have been added in the schema_salad original PR. Which kind of tests would you like to have here?
Something more functional?

@mr-c
Copy link
Member

mr-c commented Aug 24, 2022

Looking good! Can we get some tests showing off the new functionality?

Some unit tests have been added in the schema_salad original PR. Which kind of tests would you like to have here?
Something more functional?

Yeah, loading of a particular object from a CWL $graph file

@GlassOfWhiskey
Copy link
Collaborator Author

GlassOfWhiskey commented Aug 24, 2022

Yeah, loading of a particular object from a CWL $graph file

This PR is about the $schema property not the $graph property. That was another one XD.
Maybe I can copy one of the format checking conformance test files from the cwl repo and check that it is loaded correctly?

@mr-c
Copy link
Member

mr-c commented Aug 24, 2022

This PR is about the $schema property not the $graph property. That was another one XD.

Mea culpa!

Maybe I can copy one of the format checking conformance test files from the cwl repo and check that it is loaded correctly?

Yes, please :-)

@GlassOfWhiskey
Copy link
Collaborator Author

This PR is about the $schema property not the $graph property. That was another one XD.

Mea culpa!

Maybe I can copy one of the format checking conformance test files from the cwl repo and check that it is loaded correctly?

Yes, please :-)

I added just one test. I think it is enough here as this PR basically just imports an external modification which is widely tested in the schema_salad branch (see here)

@mr-c mr-c force-pushed the schema-salad-pr-583 branch from aa85db1 to 9d87144 Compare August 25, 2022 11:41
@mr-c mr-c force-pushed the schema-salad-pr-583 branch from 9d87144 to 8561612 Compare August 25, 2022 11:58
@mr-c mr-c force-pushed the schema-salad-pr-583 branch from 236490b to 59e73fc Compare August 25, 2022 12:55
@mr-c mr-c merged commit 4bad634 into main Aug 25, 2022
@mr-c mr-c deleted the schema-salad-pr-583 branch August 25, 2022 13:49
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