Skip to content

Conversation

@enryH
Copy link
Collaborator

@enryH enryH commented Jun 25, 2025

  • add description of design choices and explain the Python package setup
  • update repository to latest common changes

Choose a reason for hiding this comment

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

Can we change RasmussenLab by your_GitHub_user_name or something like that?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I actually run the template itself, to make sure everything works. so this would need a organization your_GitHub_user_name.

@albsantosdel
Copy link

Careful with having a default LICENSE. Not all packages will have a MIT license.
Also, copyright is set as Copyright (c) 2023: Jakob Nybo Nissen.

@enryH
Copy link
Collaborator Author

enryH commented Jun 27, 2025

which would be a good default LICENSE?

@enryH enryH marked this pull request as ready for review June 27, 2025 09:00
@enryH enryH requested a review from Copilot June 27, 2025 11:15
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR introduces comprehensive documentation of design decisions and updates project metadata and tooling configurations to align with the latest standards.

  • Add a root-level developing.md detailing design choices and include it in Sphinx.
  • Update linting, formatting, and build configurations in pyproject.toml and setup.cfg.
  • Refresh repository metadata (authors, license, README, Sphinx conf, CI workflow).

Reviewed Changes

Copilot reviewed 11 out of 11 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
setup.cfg Added Flake8 configuration (exclude, max-line-length, aggressive)
pyproject.toml Updated authors, dev dependencies, Ruff and Black settings
docs/index.md Added developing entry to documentation index
docs/developing.md New Sphinx include for the root developing.md
docs/conf.py Updated author metadata and documented Sphinx extensions
docs/.gitignore Ignore build artifacts (_build, reference, jupyter_execute)
developing.md Added detailed design and packaging guide
README.md Updated usage instructions, template notes, and ReadTheDocs link
LICENSE Updated copyright year and author
.readthedocs.yaml Added placeholder for apt_packages in ReadTheDocs config
.github/workflows/cicd.yml Changed lint target to src, removed PyPI credentials lines
Comments suppressed due to low confidence (5)

docs/index.md:33

  • [nitpick] The entry developing is lowercase and may be inconsistent with other titles; consider capitalizing it (e.g., Developing).
developing

developing.md:37

  • The link text references [project.toml] but the actual file is pyproject.toml; update the label to match the filename.
[`project.toml`](pyproject.toml) file is the main configuration file for the Python package

README.md:14

  • [nitpick] This bullet appears to be nested under the previous item due to extra indentation; align it with the other top-level list items.
    - also the folder `src/python_package` 

.github/workflows/cicd.yml:122

  • The PyPI publish step no longer has credentials configured; re-add the required pypi_token: ${{ secrets.PYPI_API_TOKEN }} (or appropriate inputs) so the action can authenticate.
      # register PyPI integration:

.github/workflows/cicd.yml:37

  • [nitpick] Limiting lint to src skips code in tests (and other directories); consider using ruff check . or including additional directories to ensure all code is linted.
          ruff check src

enryH and others added 2 commits June 27, 2025 13:19
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
@angelphanth
Copy link

Thank you for this template and the documentation 😃 🎉

I used the template to publish to pypi: hugging-mapper. It was my first time building on Read The Docs -- I'm pumped!!! I learned A LOT, thank you.

Some things I encountered during:

  1. (pyproject.toml) I couldn't use a requirements.txt and had to list the dependencies under [project] for pypi
  2. (.github/workflows/cicd.yml) maybe its overkill but i added an additional job to test the build before publishing. Link (git repo file) to the added job code
  3. (.github/workflows/cicd.yml) when running the tests I used -v flag for troubleshooting: python -m pytest -vvv

@enryH
Copy link
Collaborator Author

enryH commented Jun 29, 2025

Thanks Angel for trying it directly and the hints. I will test and update accordingly. I hope you enjoyed also the excitement when your Python Package was automatically published to PyPI:)

As noted by Angel using requirements.txt needed one additional change:
- add dependencies to the dynamic field

Add hint to section with dependencies
@enryH
Copy link
Collaborator Author

enryH commented Jun 30, 2025

I will add a hint in the document specifying that it's possible to add testing of installation of the package from the build distribution. This can be handy in case non code files are shipped with the package and you want to make sure that loading these works as expected. Angel tested the installation and ran pytest again using the manifest. maybe you do not even want to checkout your repo in that job so you make sure not to rely on any local package files:

  test_sdist:
    name: Test built source distribution
    needs: build_source_dist
    runs-on: ubuntu-latest
    steps:
      - uses: actions/checkout@v4 
      
      - uses: actions/download-artifact@v4
        with:
          name: artifact
          path: ./dist

      - uses: actions/setup-python@v5
        with:
          python-version: "3.11"

      - name: Install test dependencies
        run: |
          python -m pip install --upgrade pip
          pip install pytest

      - name: Install built sdist
        run: |
          pip install ./dist/*.tar.gz

      - name: Run tests
        run: python -m pytest -vvv

@sayalaruano
Copy link

Regarding choosing a license, this tool guide helps to make this decision, depending on the specificities of the project.

@enryH enryH mentioned this pull request Jul 1, 2025
- add more to .gitignore and link template
- remove line-length configuration as it is already on the default 88 of black formatter
@enryH enryH merged commit 4315f2f into main Jul 1, 2025
12 checks passed
@enryH enryH deleted the document branch July 8, 2025 13:22
enryH added a commit that referenced this pull request Jul 8, 2025
* 🎨 add flake8 and ruff line length default (to 90 characters)

* 🎨 add formatting of jupyter notebooks per default

* 📝 highlight that also a folder has to be renamed

* 📝 document what extension are used for

* 📝 document debian (ubuntu) package adding which might be needed

* 🎨 use PyPI and GitHub integration instead of secret api token

* 🙈 hide folders creating when locally building the documentation website

* 📝 document desing choices and layout of Python package

* 🎨 add copy button for code blocks per default

* 🎨 small corrections after proof-reading

* 🎨 make name generic and document it. Choose a Licence hint

* 🎨 add design document to Sphinx documentation

* 🎨 add some more common error checking

- set line-length to black default of 88 characters

* Apply suggestions from code review



* 🐛 correct python pkg configuration file

* 🐛 add missing changes for requirments.txt

As noted by Angel using requirements.txt needed one additional change:
- add dependencies to the dynamic field

Add hint to section with dependencies

* 📝 Add hint to GitHub Releases and Source distribution testing

* 🎨 follow Pasquale's advices (.gitignore + unnecessary config)

- add more to .gitignore and link template
- remove line-length configuration as it is already on the default 88 of black formatter

* 📝 add sebastians hints on other ressources

* 🎨 add license key and remove from classifiers as suggested by Sebastian

* 📝 docstring and src layout hints (noted by sebastian)

* 🐛 add missing key to pyproject.toml

---------

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
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.

5 participants