Issue 2037: extend shaclgen.py to add the suffix "Shape" to the generated SHACL shape

[robertschubert]

This PR implements the feature requested in #2037.

Fixes #2037

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 80.68%. Comparing base (f5076a6) to head (46de5f1).
Report is 5 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #2038      +/-   ##
==========================================
+ Coverage   80.67%   80.68%   +0.01%     
==========================================
  Files         107      107              
  Lines       11921    11949      +28     
  Branches     3409     3417       +8     
==========================================
+ Hits         9617     9641      +24     
- Misses       1741     1743       +2     
- Partials      563      565       +2     

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

[cmungall]

Can you make it such that appending Shape or any other suffix is opt-in and the default behavior remains the same?

[robertschubert]

Can you make it such that appending Shape or any other suffix is opt-in and the default behavior remains the same?

I will try to find the mechanism to pass CLI parameters to the generator.

[robertschubert]

Hi @cmungall
thanks for your valuable feedback. I changed the implementation as you requested. The suffix is now a cli parameter and defaults to "".

But I am struggling with the checks:

• The "build and test documentation" states: "/home/runner/work/linkml/linkml/docs/generators/shacl.rst:18: CRITICAL: Unexpected section title or transition."

  • There was no change in shacl.rst. The part where my changes come into the game is starting from the section "Command line" starting at line 87. You have a hint?

• The "build and test linkml / test (ubuntu-latest," "3.9" and "3.10") seem to have an infrastructural problem.

  • They fail with "ModuleNotFoundError: No module named 'pyshacl'"
  • Other recent PRs seem to have the same problem with those three checks.
  • Do we need another issue to create?

[sneakers-the-rat]

super weird. checking on this because i was the one who proposed making pyshacl an extra dependency here: #1992

so the cached environment for 3.9 is from here: https://github.com/linkml/linkml/actions/runs/8445452327/job/23132690282
key: venv-3.9-Linux-2130111127bb0cfd765b175f385377faa156d7790932052a2ac8d8368c099e5b

pyshacl is installed: https://github.com/linkml/linkml/actions/runs/8445452327/job/23132690282#step:6:169

it does not seem to be removed in the subsequent call to install pydantic.

these runs restore the same cache key: https://github.com/linkml/linkml/actions/runs/8473903961/job/23219220620?pr=2038#step:5:17

but it seems to say "the venv is broken" - https://github.com/linkml/linkml/actions/runs/8473903961/job/23219220620?pr=2038#step:7:9

no idea what that means.

pretty sure the fix is to do poetry run pip install 'pydantic<2' (not sure if you need --force rather than poetry add because that attempts to re-solve the environment rather than modifying a single package, which is what we want.

[sneakers-the-rat]

invalidated the caches and it looks like tests are passing now. ya we probs want to change that pydantic install command (or move ahead with deprecating pydantic 1!!!)

[robertschubert]

@sneakers-the-rat: Thanks for making the failing checks running! :)

[cmungall]

The sphinx error messages are a bit opaque but I think what is happening is that terminusdb was recently moved from a dev requirement to an extra, it needs to be added to the list of extras to install for doc building. Sorry you were caught in all this Robert, your PR has nothing to do with it! Will try and get to this later today

…

On Fri, Mar 29, 2024 at 3:21 AM robertschubert ***@***.***> wrote: @sneakers-the-rat <https://github.com/sneakers-the-rat>: Thanks for making the failing checks running! :) — Reply to this email directly, view it on GitHub <#2038 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAAMMOPYIDJQXXE3RKKDUNTY2UXBHAVCNFSM6AAAAABFMOGYMSVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDAMRXGAZTCNRXGA> . You are receiving this because you were mentioned.Message ID: ***@***.***>

[robertschubert]

Thanks @cmungall and @sneakers-the-rat
no problem. I'm glad you're helping.
Must say I like this community here :)
Hope my PR helps. There is no breaking change now.

[robertschubert]

I'm in easter holiday now for a week but I will of course have a look at the progress here. Could be that I am not able to respond early ;) have a good time!

[sneakers-the-rat]

figured it out - it was from having the empty string as the default and having "show default" set to True

can be resolved by removing show_default in the suffix click option or by making the default None

imo optional params should default to None so it's possible to tell when they aren't passed vs. when they should be filled with a default. so that would be something like

@click.option(
    "-s",
    "--suffix",
    help="Use --suffix to append given string to SHACL class name (e. g. --suffix Shape: Person becomes PersonShape).",
)

and then

if self.suffix is not None:
    class_url += suffix

but that's just a style thing i don't use shaclgen so up to you!!!!

[robertschubert]

@sneakers-the-rat Thanks for the hint. Changed it and that step is fine now :)
After all checks are completed, I will set this PR to "ready".

[robertschubert]

ready for final review

[paul121]

Thank you for adding this!