Skip to content

Harmonize with new version of econ-project-templates. #8

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 22 commits into from
Feb 14, 2022

Conversation

hmgaudecker
Copy link
Contributor

@hmgaudecker hmgaudecker commented Feb 4, 2022

Closes #5.

Sync with OpenSourceEconomics/econ-project-templates#98

  • description re version in {{cookiecutter.project_slug}}/config.py unclear, see comment below. (Removed)
  • pytask.ini vs. tox.ini (why both?) (Removed pytask.ini)
  • add setup.py ? Mentioned in best practices of pytask docs. In any case, harmonise. Only keep pyproject.toml and setup.cfg as explained in the tutorial.
  • add empty docs ?
  • Top-level README.rst :target: https://pypi.org/project/pytask correct? (Fixed)
  • Why is ISSUE_TEMPLATE in capital letters and pull_request_template.md not? Github requirement? I almost missed the latter... (use uppercase folders to store multiple templates)
  • Document PR in CHANGES.rst.

@codecov
Copy link

codecov bot commented Feb 9, 2022

Codecov Report

Merging #8 (599a917) into main (b42fe01) will decrease coverage by 16.32%.
The diff coverage is 43.33%.

❗ Current head 599a917 differs from pull request most recent head 17ac7d7. Consider uploading reports for the commit 17ac7d7 to get more accurate results

Impacted file tree graph

@@             Coverage Diff             @@
##             main       #8       +/-   ##
===========================================
- Coverage   97.22%   80.89%   -16.33%     
===========================================
  Files           2        4        +2     
  Lines          72       89       +17     
===========================================
+ Hits           70       72        +2     
- Misses          2       17       +15     
Flag Coverage Δ
end_to_end 80.89% <43.33%> (-16.33%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
{{cookiecutter.project_slug}}/docs/source/conf.py 28.57% <28.57%> (ø)
...slug}}/src/{{cookiecutter.project_slug}}/config.py 75.00% <50.00%> (ø)
...ug}}/src/{{cookiecutter.project_slug}}/__init__.py 80.00% <80.00%> (ø)
tests/test_cookie.py 100.00% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update b42fe01...17ac7d7. Read the comment docs.

@hmgaudecker
Copy link
Contributor Author

@tobiasraabe -- related to pytask-dev/pytask#217, is it enough to just remove setup.py again?

@tobiasraabe
Copy link
Member

Yes, and require pip >= 21.1.

@tobiasraabe
Copy link
Member

tobiasraabe commented Feb 10, 2022

What about the last open point? I am not sure about having predefined docs, since it can be tedious to keep it up-to-date with sphinx.

On the other hand, there are some things which are always handy and you might want to consider for econ-project-templates.

@hmgaudecker
Copy link
Contributor Author

hmgaudecker commented Feb 10, 2022

What about the last open point? I am not sure about having predefined docs, since it can be tedious to keep it up-to-date with sphinx.

Just a very small stub? The readthedocs template not make too much sense otherwise? Will have to keep up with a little bit over on econ-project-templates anyhow...

On the other hand, there are some things which are always handy and you might want to consider for econ-project-templates.

* A automatically generated API page with https://github.com/readthedocs/sphinx-autoapi which renders to https://pytask-dev.readthedocs.io/en/latest/api.html

* a stub for `changes.rst`

The latter is included, the former we should include!

@hmgaudecker
Copy link
Contributor Author

Anyhow, I don't expect to finish this PR before OpenSourceEconomics/econ-project-templates#98 is basically done. Unless you prefer to merge and open a new one should we see stuff in the meantime.

My goal is that econ-project-templates ends up as a superset of this one.

@tobiasraabe
Copy link
Member

@hmgaudecker, I am fine with opening more PRs if changes are necessary. This one is already too big. If you dont have any objections, I will merge it.

@hmgaudecker
Copy link
Contributor Author

Sounds great -- you are the boss, anyhow!

Looking at the docs, the only question is whether we can use the automated versioning machinery for the documentation of the template itself? That is, something like

from importlib.metadata import version

in docs/source/conf.py instead of manually setting version / release?

Other than that, this looks great.

The only reason I might feel like keeping this PR open is to increase the chances of making improvements here instead of implementing workarounds in the corresponding econ-project-templates PR. @timmens, please feel free to open another one as you see fit! (@tobiasraabe, do you mind adding Tim to this repo?)

@tobiasraabe
Copy link
Member

I implemented versioning with setuptools-scm for the cookiecutter itself. Nice catch!

Apart from that, I am happy to accept more PRs and merge this one now. I'll also add @timmens to the repo. In the future, I would like people who are more loosely related than Tim to fork the repo.

@tobiasraabe tobiasraabe changed the title [WIP] Harmonize with new version of econ-project-templates Harmonize with new version of econ-project-templates. Feb 14, 2022
@tobiasraabe tobiasraabe merged commit 6f20a8d into main Feb 14, 2022
@tobiasraabe tobiasraabe deleted the harmonize-with-econ-project-templates branch February 14, 2022 12:16
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.

ENH: Fix some leftovers.
2 participants