Skip to content

Conversation

richarddushime
Copy link
Contributor

@richarddushime richarddushime commented Jul 30, 2025

Before submitting a pull request (PR), please read the contributing guide.

Please fill out as much of this template as you can, but if you have any problems or questions, just leave a comment and we will help out :)

Description

This PR updates the documentation configuration (docs/source/conf.py) to retrieve the version using importlib.metadata.version.

What is this PR

  • Bug fix
  • Addition of a new feature
  • Other

Why is this PR needed?

This change aligns with the recommended usage from setuptools-scm documentation for retrieving the version. It replaces the direct call to setuptools_scm.get_version with importlib.metadata.

What does this PR do?

This PR replaces the setuptools_scm version retrieval in docs/source/conf.py with the use of importlib.metadata. The get_version function is imported from importlib.metadata

References

Please reference any existing issues/PRs that relate to this PR.

Fixes #144
Fixes #102

How has this PR been tested?

Please explain how any new code has been tested, and how you have ensured that no existing functionality has changed.

Local build

Is this a breaking change?

If this PR breaks any existing functionality, please explain how and why.
No

Does this PR require an update to the documentation?

If any features have changed, or have been added. Please explain how the
documentation has been updated.
No

Checklist:

  • The code has been tested locally
  • Tests have been added to cover all new functionality
  • The documentation has been updated to reflect any changes
  • The code has been formatted with pre-commit

@niksirbi niksirbi requested a review from sfmig July 31, 2025 09:25
@adamltyson
Copy link
Member

@richarddushime, would it be possible to add this change to the cookiecutter template itself?

@richarddushime
Copy link
Contributor Author

@richarddushime, would it be possible to add this change to the cookiecutter template itself?

yes and will cover also this #102 may need to test(I added -e . in the reqs)

Copy link
Contributor

@sfmig sfmig left a comment

Choose a reason for hiding this comment

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

Hi @richarddushime, nice to see you again! 😁

Just a couple of small inline comments and a suggestion re the contribution docs.

In the local testing section, I think it would be worth highlighting that the pip install -r docs/requirements.txt needs to be run from the root directory of the repo. Maybe some confident users are tempted to pip install -r requirements.txt from the docs dir, but this will fail with the new -e . line with a not-super-explicit error message. Would you mind updating that section of the docs too as part of this PR?

Additionally, if you can link issue 102 to this PR that would be great. If you use "Fix 102" in the PR description it will be automatically closed when the PR is merged, see here.

Thanks for the great work!

@adamltyson
Copy link
Member

FYI @sfmig you can also link an issue to a PR using the "Development" tab on the right hand side.

@sfmig
Copy link
Contributor

sfmig commented Aug 1, 2025

@adamltyson right, that is an alternative! thanks

@richarddushime
Copy link
Contributor Author

Hi @richarddushime, nice to see you again! 😁

Hi @sfmig , great to be here! 😄
Thanks for the welcome

Copy link
Contributor

@sfmig sfmig left a comment

Choose a reason for hiding this comment

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

I think this is good to go, thank you @richarddushime!

@sfmig sfmig merged commit 72167c3 into neuroinformatics-unit:main Sep 8, 2025
10 checks passed
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.

Use importlib.metadata to retrieve package version as recommended sphinx builds locally but not via github action
3 participants