Conversation
h-chmeruk
added
documentation
|
|
||
| # Set the OS, Python version and other tools you might need | ||
| build: | ||
| os: ubuntu-24.04 |
There was a problem hiding this comment.
I am not sure which version we want to use as the default. The old cookiecutter template had ubuntu-22.04, but maybe it's time to update the version.
| "license": "MIT", | ||
| "copyright_statement": "2025, The pyfar developers", | ||
| "project_short_description": "my_project_short_description", | ||
| "python_versions": "['3.9', '3.10', '3.11', '3.12', '3.13', '3.14']", |
There was a problem hiding this comment.
I have also added this line to the standard test configuration, since we will be updating the python_versions parameter in the copier.yml in the future. So that the tests do not fail and we do not have to adjust them, I added a default value for the python_versions parameter.
| {{ python_versions[min_index:] }} | ||
|
|
||
| default_python_version: | ||
| when: false |
There was a problem hiding this comment.
Currently, because of the when: False attribute, this parameter is not included in the questionnaires and always takes the default value. Please let me know if you think this parameter should be displayed to the user when creating/updating a project.
There was a problem hiding this comment.
maybe you can discuss this point in the weekly, both options would work for me.
advatange of always using the latest version is that we dont need to worry about it.
There was a problem hiding this comment.
yeah, thank you, I could bring it up in the meeting today :)
There was a problem hiding this comment.
We have decided to leave the parameter unchanged so that the latest python version is always used as the parameter value
copier.yml
Outdated
| {% set min_index = python_versions.index(minimum_python_version) %} | ||
| {{ python_versions[min_index:] }} | ||
|
|
||
| default_python_version: |
There was a problem hiding this comment.
Nice, I would vote for latest_python_version
copier.yml
Outdated
| default: "numpy,scipy" | ||
| spaces between them. These dependencies will be included in the pyproject.toml | ||
| and in the .readthedocs.yml files. | ||
| default: "numpy,scipy,libsndfile1" |
There was a problem hiding this comment.
The new dependency is not a Python package, it is a systems package and is installed via apt get on eg Linux. We can remove it here and add an other dependency list, eg apt_packages, this will also be used in cicleci config.
… from the code review by @ahms5
h-chmeruk
force-pushed
the
add-readthedocs-file
branch
from
2b6e2ce to
4337713
Compare
github-project-automation
bot
moved this from Agenda
to Require review
in Weekly Planning
ahms5
left a comment
ahms5
left a comment
There was a problem hiding this comment.
Looks good to me, maybe you can discuss weather we want to always use the lastest version by as a python defualt version in the meeting today. Ive shifted it to agenda. I wouldnt be able to make it today.
copier.yml
Outdated
| Enter the APT packages of this project, separate them with commas and do not leave any | ||
| spaces between them. |
There was a problem hiding this comment.
| Enter the APT packages of this project, separate them with commas and do not leave any | |
| spaces between them. | |
| Enter the required apt packages for this project, separate them with commas and do not leave any | |
| spaces between them. The dependencies are installed before each CI/CD workflow and readthedocs build via apt-get on a Linux system. |
ahms5
requested review from
a team,
ahms5 and
hoyer-a
and removed request for
ahms5
github-project-automation
bot
moved this from Require review
to Reviewer Approved
in Weekly Planning
github-project-automation
bot
moved this from Require review
to Done
in Weekly Planning
4 participants
Which issue(s) are closed by this pull request?
Closes #36
Changes proposed in this pull request:
default_python_versionto thecopier.ymlfile. The parameter takes the last version from thepython_versionsparameter as its default value.test_content_readthedocs(), extendedtest_generated_file_exists().