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

Parsing cwl metadata #702

Merged
merged 26 commits into from
Sep 12, 2024
Merged

Parsing cwl metadata #702

merged 26 commits into from
Sep 12, 2024

Conversation

Nazim-crim
Copy link
Contributor

@Nazim-crim Nazim-crim commented Aug 29, 2024

Changes

@Nazim-crim Nazim-crim self-assigned this Aug 29, 2024
@github-actions github-actions bot added ci/doc Issue related to documentation of the package ci/operations Related to CI operations (actions, execution, install, builds, etc.) ci/tests Tests of the package and features feature/CWL Issue related to CWL support feature/docker Issue related to Docker application package execution. feature/oas Issues related to OpenAPI specifications. process/wps3 Issue related to WPS 3.x (REST-JSON) processes support labels Aug 29, 2024
Copy link

codecov bot commented Aug 29, 2024

Codecov Report

Attention: Patch coverage is 96.42857% with 1 line in your changes missing coverage. Please review.

Project coverage is 85.95%. Comparing base (9788880) to head (b76cfd1).
Report is 27 commits behind head on master.

Files with missing lines Patch % Lines
weaver/processes/wps_package.py 96.00% 0 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master     #702      +/-   ##
==========================================
+ Coverage   85.94%   85.95%   +0.01%     
==========================================
  Files          80       80              
  Lines       19551    19578      +27     
  Branches     3045     3055      +10     
==========================================
+ Hits        16803    16829      +26     
  Misses       1983     1983              
- Partials      765      766       +1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

Thanks for submitting a PR.
Make sure that you have added tests to validates new features or bugfixes. Also, ensure that existing test suites are still working with the change using the commit test statuses.

Copy link
Collaborator

@fmigneault fmigneault left a comment

Choose a reason for hiding this comment

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

Need to add an entry to CHANGES.md (same items as the PR description).

weaver/wps_restapi/swagger_definitions.py Outdated Show resolved Hide resolved
tests/processes/test_wps_package.py Outdated Show resolved Hide resolved
tests/processes/test_wps_package.py Outdated Show resolved Hide resolved
tests/processes/test_wps_package.py Outdated Show resolved Hide resolved
tests/processes/test_wps_package.py Show resolved Hide resolved
tests/processes/test_wps_package.py Outdated Show resolved Hide resolved
weaver/processes/wps_package.py Outdated Show resolved Hide resolved
weaver/processes/wps_package.py Outdated Show resolved Hide resolved
weaver/typedefs.py Outdated Show resolved Hide resolved
Copy link
Collaborator

@fmigneault fmigneault left a comment

Choose a reason for hiding this comment

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

Just editorial edits left.

CHANGES.rst Outdated
Comment on lines 34 to 35
(fixes `#463 <https://github.com/crim-ca/weaver/issues/463>`_)
- Add more metadata mapping details in documentation (fixes `#613 <https://github.com/crim-ca/weaver/issues/613>`_)
Copy link
Collaborator

@fmigneault fmigneault Sep 12, 2024

Choose a reason for hiding this comment

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

Add the trailing . instead of the space, for each item.

Comment on lines 1318 to 1328
Additional compatible elements found under ``metadata`` in :term:`WPS` Context

Parameters in :term:`CWL` Context:

- ``s:author``
- ``s:citation``
- ``s:codeRepository``
- ``s:contributor``
- ``s:dateCreated``
- ``s:license``
- ``s:releaseNotes``
Copy link
Collaborator

Choose a reason for hiding this comment

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

For this, I believe it should be added directly to the metadata line that is already present in the table. Basically, just extend the right side (where $schemas/$namespace is indicated) with the mention of those fields.

You can also add the [#cwl_schemaorg]_ footnote reference (as in the s:keywords case), that points to the CWL metadata example below, which contains all of those fields.

@@ -1312,6 +1312,20 @@ Below is a list of compatible elements.
+-----------------------------------------+----------------------------------------------------------+
| ``abstract``/``description`` | ``doc`` |
+-----------------------------------------+----------------------------------------------------------+
| ``version`` | ``s:version``/``s:softwareVersion`` |
Copy link
Collaborator

Choose a reason for hiding this comment

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

Could also use the [#cwl_schemaorg]_.

@fmigneault fmigneault self-requested a review September 12, 2024 18:35
@fmigneault
Copy link
Collaborator

@Nazim-crim Thanks!

@fmigneault fmigneault merged commit b41c211 into master Sep 12, 2024
48 checks passed
@fmigneault fmigneault deleted the parsing-metadata branch September 12, 2024 18:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ci/doc Issue related to documentation of the package ci/operations Related to CI operations (actions, execution, install, builds, etc.) ci/tests Tests of the package and features feature/CWL Issue related to CWL support feature/docker Issue related to Docker application package execution. feature/oas Issues related to OpenAPI specifications. process/wps3 Issue related to WPS 3.x (REST-JSON) processes support
Projects
None yet
2 participants