Skip to content

Proposing add_formal_parameter function #221

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 13 commits into from
May 9, 2025

Conversation

LauLauThom
Copy link
Contributor

Following yesterday's call, here is a small PR proposing the addition of a function to facilitate the definition of FormalParameters in a workflow crate.

It makes use of type hints, which is available from python 3.5, support could be added for earlier version by adding the typing package to the list of dependencies, checking the version of python to only install if needed.

I am happy to write tests and documentation for this function if you think that's a valid addition.
Currently it can be tested with

from rocrate.rocrate import ROCrate
from pprint import pprint

crate = ROCrate()
formal_parameter = crate.add_formal_parameter(identifier = "my_param",
                                              additionalType = "Text",
                                              name = "my_param",
                                              description = "",
                                              valueRequired = False,
                                              defaultValue = "")
pprint(formal_parameter.as_jsonld())

Actually for testing I typically install the package in editable mode like so
pip install -e .
but I got an exception that it's not possible

DEPRECATION: Legacy editable install of rocrate==0.13.0 from file:///Users/thomasl/Documents/repos/ro-crate-py (setup.py develop) is deprecated. pip 25.1 will enforce this behaviour change. A possible replacement is to add a pyproject.toml or enable --use-pep517, and use setuptools >= 64. If the resulting installation is not behaving as expected, try using --config-settings editable_mode=compat. Please consult the setuptools documentation for more information. Discussion can be found at https://github.com/pypa/pip/issues/11457

So I was wondering how you test it usually.
I also had issues with test discovery, but that's maybe something with my VScode config.

Cheers !

@LauLauThom LauLauThom marked this pull request as draft April 25, 2025 09:51
@simleo
Copy link
Collaborator

simleo commented May 5, 2025

I've found some issues with the PR:

  • Type hints: the rest of the library is currently not using them
  • Indentation should be consistent with the rest of the library
  • name is not a required argument, only additionalType is (see https://w3id.org/ro/wfrun/workflow)
  • The method should also take properties as an argument, so that additional properties can be specified
  • additionalType can take other values (e.g. Collection, Float), but there is no actual allowed types list in the spec, so better not mention them

I've applied these changes in 4ee784c to give the idea.

Please add a unit test to test_wrroc.py, using test_add_action as an example.

I could not reproduce the editable mode issue, on my PC it works with pip-25.1.1. This blog post has some info. Maybe your setuptools version is too old? I've added pyproject.toml with the content suggested in the blog post, I hope that helps you fix the problem.

@LauLauThom
Copy link
Contributor Author

Thanks @simleo for reviewing the PR, I will try adding the tests in the coming days.
I could install in editable mode with the pyproject.toml, thanks !

Regarding the name field, the bioschema specs v1.0 (the one referenced by the field conformsTo) mentions it as mandatory, and so does the spec for ROCrate 1.2 RC1

If complying with the Bioschemas FormalParameter profile, the contextual entities for FormalParameter, referenced by input or output, MUST describe name.

@simleo
Copy link
Collaborator

simleo commented May 6, 2025

Regarding the name field, the bioschema specs v1.0 (the one referenced by the field conformsTo) mentions it as mandatory, and so does the spec for ROCrate 1.2 RC1

Yes, but neither the Workflow Run Crate profile nor the base spec require strict conformance to the Bioschemas spec. However, we are adding the conformsTo automatically, so I changed name to be a required argument in 0e7995c. Users that need to add a parameter that does not conform to Bioschemas can fall back to using ContextEntity, or override relevant properties with the properties argument.

@simleo simleo marked this pull request as ready for review May 7, 2025 08:25
@simleo
Copy link
Collaborator

simleo commented May 7, 2025

@LauLauThom thanks for the contribution! before merging I need you to update author and copyright info. Author info is in CITATION.cff, setup.py and rocrate/__init__.py. Copyright info is in README.md, rocrate/__init__.py and Python file headers. You can see 09bff64 and 04128fa for an example of similar changes.

@LauLauThom
Copy link
Contributor Author

Done, I am not sure I can attend the call tomorrow though, I have a training until 4 that might go overtime.
In 2 weeks hopefully, cheers.

@simleo
Copy link
Collaborator

simleo commented May 7, 2025

Done

No, the copyright info is still missing

@LauLauThom
Copy link
Contributor Author

sorry, I had overlooked this, I mentioned the city in addition to the country, because the EMBL has different sites (well there is just one in germany but still)

@simleo simleo merged commit 9c0d8a0 into ResearchObject:master May 9, 2025
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