Closed ddelange closed 3 years ago
@ddelange The elaboration in this PR is really good and well-organized. Which I learned a lot! Looking forward to your contributions! ❤️
Hey @ddelange thanks so much for your PR! I would've loved to merge it - cookiecutter has been a thorn in my/our side for so long now. And you did a wonderful job explaining what you did and why.
We're looking at options right now for replacing cookiecutter with something more suitable and flexible. Would love your feedback on that once it goes live
Hi!
I looked at the CHANGELOG and couldn't quickly spot breaking changes until 1.0.7 versus 0.8.2 (are there any that need to be considered here, since the major version was bumped?). I've made the changes below assuming semantic versioning (dropping patch version here affects both dockerhub and pypi, see below), but as it's a major version bump, this PR might need some extra eyes to ensure the bump to v1 is fine.
Changes:
cookiecutter.json
from 0.8.2 to 1.0 (but entering a patch version will also still work).pip install jina==1.0.7.*
successfully installs 1.0.7, as the trailing.*
is explicitly allowed (see link above), and can be hard-coded in requirements.txt. The alternative was a compatible release clause, but asjina~=1.0
is equivalent tojina>=1.0,<2
, pip would allow e.g. 1.1, meaning divergence with the docker tag.nlp
task type)framework
extra, to replace the seemingly deprecatedtensorflow
extranlp
extra to get torch instead of hard coding/pinning here.transformers
by default (which fetches torch), rather rely onnlp
extra.Side question: do you already have plans for some checks for this repo? Maybe something like:
pre-commit run --all-files
and ensure the hooks will start passing after that, to ensure there are no weird things happening in the PRapp.py