feat(template): Added hatchling as an option for build-system #144

ayeankit · 2023-07-07T22:30:20Z

Pull Request description

Activate option in TUI to hatchling
Adding documentation about hatchling
Adding hatchling as a build-system:
Creating a hatch-pyproject.toml
Editing post_gen_project.py
Creating a smoke test (build-system.sh)
Editing cookicutter.json

Fixes #69

How to test these changes

...

Pull Request checklists

This PR is a:

bug-fix
new feature
maintenance

About this PR:

it includes tests.
the tests are executed on CI.
the tests generate log file(s) (path).
pre-commit hooks were executed locally.
this PR requires a project documentation update.

Author's checklist:

I have reviewed the changes and it contains no misspelling.
The code is well commented, especially in the parts that contain more complexity.
New and old tests passed locally.

Additional information

Reviewer's checklist

Copy and paste this template for your review's note:

## Reviewer's Checklist

- [ ] I managed to reproduce the problem locally from the `main` branch
- [ ] I managed to test the new changes locally
- [ ] I confirm that the issues mentioned were fixed/resolved .

Saransh-cpp

Thanks, @ayeankit! Looks good overall. Some comments below. You'll also have to take a look at the failing tests.

docs/guide.md

Saransh-cpp · 2023-07-08T15:21:49Z

src/scicookie/{{cookiecutter.project_slug}}/build-system/hatchling-pyproject.toml

@@ -0,0 +1,135 @@
+[build-system]
+requires = ["hatchling>=1.17.1", "hatch-vcs>=0.3.0"]


Hmm, if we are keeping VCS versioning as the default one, it might make sense to add a version.pyi file. But as I look at other backends, in particular setuptools, there is no usage of setuptools_scm, which means we might not want the VCS versioning by default?

This would require adding something like this -

[tool.hatch] version.source = "vcs" build.hooks.vcs.version-file = "some/path/here/version.py"

@Saransh-cpp I removed the vcs version as the default one. Is it necessary to add this snippet? (as vcs is not in use)

[tool.hatch] version.source = "vcs" build.hooks.vcs.version-file = "some/path/here/version.py"

Not now! But we might need to explore the path to provide a user with an option to turn on VCS versioning. cc: @xmnlab.

docs/guide.md

ayeankit · 2023-07-10T12:49:00Z

@xmnlab Can you please review it once ?

Saransh-cpp

Some design issues -

Each build backend library is kind of a combination of a frontend (CLI tool) and a backend (the actual build-backend), for instance, there is flit and flit-core, similarly poetry and poetry-core, and the messiest one, setuptools, that tries to be both a backend and a frontend simultaneously.

So, scicookie uses the "frontend" names for the build-backends which is completely acceptable, and hence, we should use the name hatch and not hatchling throughout the codebase. Remember, hatch is the CLI frontend, and hatchling is the backend. We use flit everywhere to mention the flit build-backend and not flit-core, similarly, we should use hatch everywhere to mention the hatchling build-backend.

This should also fix your tests as your PR confuses hatch and hatchling at a lot of places.

cc: @xmnlab (to fix a convention for naming the backends). Frontend names sound good to me, https://github.com/scientific-python/cookie uses the frontend names too.

Other general tips 🙂

Please have some meaningful messages in your commits. "Update x" makes it really hard to guess what was updated by just looking at the message.
Writing "Closes Add hatchling as a build-system option #69" or "Fixes Add hatchling as a build-system option #69" in the PR description will automatically close the issue for which the PR was opened. This does make a maintainer's life easy haha.
I found a few contradictions in your PR description and the actual files changed.

src/scicookie/{{cookiecutter.project_slug}}/Makefile

src/scicookie/{{cookiecutter.project_slug}}/conda/dev.yaml

src/scicookie/cookiecutter.json

docs/guide.md

src/scicookie/hooks/post_gen_project.py

src/scicookie/{{cookiecutter.project_slug}}/conda/dev.yaml

src/scicookie/hooks/post_gen_project.py

src/scicookie/{{cookiecutter.project_slug}}/Makefile

tests/smoke/base.sh

Saransh-cpp · 2023-07-10T17:41:02Z

src/scicookie/{{cookiecutter.project_slug}}/build-system/hatchling-pyproject.toml

@@ -0,0 +1,135 @@
+[build-system]
+requires = ["hatchling>=1.17.1", "hatch-vcs>=0.3.0"]


Not now! But we might need to explore the path to provide a user with an option to turn on VCS versioning. cc: @xmnlab.

xmnlab · 2023-07-10T17:57:07Z

#144 (comment)

@Saransh-cpp , totally agree! for now we have just semantic release, but probably we should have an option in a near future to select between semantic-release or vcs aproach.

Saransh-cpp · 2023-07-11T13:51:28Z

tests/smoke/base.sh

@@ -49,6 +49,8 @@ elif command -v meson &> /dev/null; then
  pip install -e .
 elif command -v pdm &> /dev/null; then
  pdm install
+elif command -v hatch &> /dev/null; then
+  pip install -e .


I think this should be hatch install? Was that giving any errors?

maybe I am wrong, I am not a hatch/hatchling user
but it seems that hatch doesn't have a command for install just for building
and it seems that for executing it in local mode we should run tests with something like hatch run pytest .

so using pip install -e here would be a workaround .. but TBH I don't know if that works
another option would be to build it and install it from the packaged created by build
but it probably will not install the dev dependencies (for test and for documentation)

so, not sure yet what could be the best workflow for this to keep it generic ...
otherwise we will need to create an environment variable called for example COMMAND_PREFIX
and add it to all the other commands that will need to have access to the dependencies
and for hachling we will set it to hatch run

for example:

COMMAND_PREFIX="hatch run" $COMMAND_PREFIX pytest

but the error on CI looks like some syntax error on pyproject.toml

Ah yes, thanks for the clarification!

Saransh-cpp · 2023-07-13T23:01:54Z

src/scicookie/{{cookiecutter.project_slug}}/build-system/hatch-pyproject.toml

+  "Topic :: Scientific/Engineering",
+  "Typing :: Typed",
+]
+dynamic = ["version"]


This probably won't fix the TOML error, but you must specify the file from which the version has to be read if you are setting it up dynamically - https://hatch.pypa.io/latest/version/#configuration -

[tool.hatch.version] path = "path/to/version/file.py"

hatch requires having either version = "x.x.x" or __version__ = "x.x.x" in the version file (in our case, __init__.py) (without the type signature (: src). We can update the regex to adapt to this project. If that does not work then we can have a condition in __init__.py to check for hatch and change the way version and __version__ are declared.

Hi, I have a question here. In the __init__.py we don't have directly the version value but by the get_version function. How should this be configured then?

@Anavelyz
using get_version instead of the direct value maybe is not super necessary with semantic release .. but it would be important with vcs that can also calculate a version from the commit hash. but not sure about hatch

hatch requires having either version = "x.x.x" or version = "x.x.x"
@Saransh-cpp , you mean a literal "x.x.x" ? or just a literal string in this format?

btw, as a general thought, the typing annotation shouldn't be a problem because if we have something like a = "mystring" without typing annotation it should consider it as str anyway. not sure why hatch would fail using the version variable with typing annotation.

the typing annotation shouldn't be a problem because if we have something like a = "mystring" without typing annotation it should consider it as str anyway. not sure why hatch would fail using the version variable with typing annotation.

I think the default regex that hatch uses to find the version string does not consider the typing annotation. It just looks for the variable version or __version__ which is assigned a string without any typing annotation - https://hatch.pypa.io/latest/version/#configuration.

Hi, I have a question here. In the init.py we don't have directly the version value but by the get_version function. How should this be configured then?

Also, given that hatch uses regex to find the version defined in a file, using a function (get_version) might not work as the function will never be executed.

@Saransh-cpp , you mean a literal "x.x.x" ? or just a literal string in this format?

A string in "x.x.x" format, sorry for the confusion.

Note how scientific-python/cookie also defines version just as a string without any type annotation -

https://github.com/scientific-python/cookie/blob/60cc884598c5abd8d300c293c528ebf0ffab2902/%7B%7Bcookiecutter.project_name%7D%7D/src/%7B%7Bcookiecutter.__project_slug%7D%7D/__init__.py#L13

Then they link that file in pyproject.toml, similar to the suggestion I made (and I think has been added in this PR) -

https://github.com/scientific-python/cookie/blob/60cc884598c5abd8d300c293c528ebf0ffab2902/%7B%7Bcookiecutter.project_name%7D%7D/pyproject.toml#L206

@ayeankit regarding the commit messages again, could you add some relevant messages to the commits that you are creating so we know what all you are trying while debugging? This would make reviewing this a bit easier too.

src/scicookie/{{cookiecutter.project_slug}}/build-system/hatch-pyproject.toml

Saransh-cpp

@ayeankit please take a look at the comments above and below. This PR currently does not implement hatchling as a backend correctly and the failing CI is right.

Saransh-cpp · 2023-07-14T00:05:55Z

src/scicookie/{{cookiecutter.project_slug}}/build-system/hatch-pyproject.toml

+{% endif %}
+{%- endif -%}{#- end of use_pytest -#}
+{%- if cookiecutter.use_hypothesis == "yes" -%}
+hypothesis = "^6.0"


Suggested change

hypothesis = "^6.0"

hypothesis = "^6.0"

not sure .. but I guess that hatch doesn't work with ^
so , ^6.0 should be >=6.0,<7.0

Oh yes, I completely missed that. That looks poetry.

src/scicookie/{{cookiecutter.project_slug}}/build-system/hatch-pyproject.toml

Co-authored-by: Saransh Chopra <saransh0701@gmail.com>

…d changed dev-dependence

…-pyproject.toml Co-authored-by: Saransh Chopra <saransh0701@gmail.com>

Saransh-cpp

Thanks, @ayeankit! I don't think the package is actually being installed in the CI; making the passing of CI a false positive. See comments below.

Saransh-cpp · 2023-07-21T14:21:47Z

tests/smoke/base.sh

@@ -40,7 +40,7 @@ export PATH=$(echo $PATH| sed -E "s/[^:]+\/micromamba\/[^:]+//g")
 export PATH=$(echo $PATH| sed -E "s/[^:]+\/anaconda3\/[^:]+//g")
 export PATH="${CONDA_PREFIX}:${CONDA_PREFIX}/bin:$PATH"
 echo "[II] included env conda to the PATH"
-
+COMMAND_PREFIX= 


Added here by mistake?

actually it is not necessary, because bash will consider any non declared variable as empty .. but I think it is good (as a "code documentation") to have it declared and assigned to empty at the beginning ... so if anyone wants something similar they will use the same variable

Thanks for the clarification! Did not know that!

Saransh-cpp · 2023-07-21T14:22:20Z

tests/smoke/base.sh

+elif command -v hatch &> /dev/null; then
+  COMMAND_PREFIX="hatch run"


I think this should still be pip install --editable .. We do need to check if the package is being installed correctly. The current passing of CI might be a false positive.

@Saransh-cpp I tried using pip install --editable . but it didn't work out. @xmnlab has suggested me to use this method for installing hatch.

yep .. hatch has a very specific workflow ... I am not an expert though ... but it seems that it doesn't work with pip install .. and all the commands that depends on the dependencies need to be executed with hatch run ... maybe there is a way to have a better workflow with hatch ... but as I mentioned I am not an expert .. feel free to suggest a better approach .. to be honest I don't like the current one .. but at least it was a way we found to have it working

That sounds weird 🙂

I install all my hatch-based packages with pip install . and it works fine for me. We might want to look at this in the future.

Saransh-cpp · 2023-07-21T14:22:50Z

tests/smoke/base.sh

+$COMMAND_PREFIX ipython kernel install --name "python3" --user
+
+$COMMAND_PREFIX pre-commit install

-pre-commit install
-pre-commit run --all-files --verbose
+$COMMAND_PREFIX pre-commit run --all-files --verbose

-make docs-build
+$COMMAND_PREFIX make docs-build


Were the tests failing when not using "COMMAND_PREFIX"?

it seems that everything worked

oh sorry .. maybe I miss understood your question .. when not using command prefix (empty variable) for the other backend works fine .. but for hatch .. it requests hatch run

tests/smoke/build-system.sh

src/scicookie/__init__.py

Saransh-cpp · 2023-07-21T14:28:04Z

src/scicookie/{{cookiecutter.project_slug}}/build-system/hatch-pyproject.toml

+  "Topic :: Scientific/Engineering",
+  "Typing :: Typed",
+]
+version = "{{ cookiecutter.project_version }}"


To be honest, I am not a big fan of static versioning and neither is this repository. It looks like all the Pythonic backends (except poetry, probably because it does not support it by default) in this repository use dynamic versioning, which makes it natural to make Hatch use dynamic versioning too.

The final call would be of the maintainers.

In parts I agree about the static vs dynamic versioning
but using semantic-release, although you will end up with the versions static there, but you can use semantic-release-replace to update the versions automatically. and semantic release is a full stack for releasing ... different than the scm approaches ... maybe it would be possible to use semantic release + any scm approach .. but I never tested it to be honest. but also IIRC poetry doesn't have a good alternative for scm).

with all this said, what I meant is: for now it uses semantic release by default ...
so what we would need to have is (not for this PR):

an option for static or dynamic versioning

and an option for releasing tool, and can start with just semantic-release

for this PR I think that static versioning is ok, so we can keep consistency with all the other build systems and we can open an issue about that and start some discussions to check what we need for supporting both static and dynamic options.

PS: the idea is to be open to different approach as much as possible .. and we can have different "profiles" that could be more opinionated

Saransh-cpp · 2023-07-21T14:28:33Z

src/scicookie/{{cookiecutter.project_slug}}/build-system/hatch-pyproject.toml

+[tool.hatch.version]
+{# keep this line here #}
+{%- if cookiecutter.project_layout == 'flat' -%}
+path = "{{ cookiecutter.package_slug }}/__init__.py"
+{% elif cookiecutter.project_layout == 'src' -%}
+path = "src/{{ cookiecutter.package_slug }}/__init__.py"
+{% endif %}


This won't be required if we end up using static versioning for Hatch.

@Saransh-cpp could you add more information here please?
I am not following you, sorry

Sorry for the confusion. These lines are only required when we use dynamic version in pyproject.toml -

dynamic = ["version"]

but as this PR now uses static version -

scicookie/src/scicookie/{{cookiecutter.project_slug}}/build-system/hatch-pyproject.toml

Line 37 in 32704a5

version = "{{ cookiecutter.project_version }}"

the whole [tool.hatch.version] section is basically redundant.

Co-authored-by: Saransh Chopra <saransh0701@gmail.com>

ayeankit · 2023-07-21T16:12:29Z

Finally, all the test cases pass!! @xmnlab Can you review it and merge it, please? Thank you.

github-actions · 2023-07-24T13:15:47Z

🎉 This PR is included in version 0.4.0 🎉

The release is available on:

0.4.0
GitHub release

Your semantic-release bot 📦🚀

Saransh-cpp requested changes Jul 8, 2023

View reviewed changes

Saransh-cpp requested changes Jul 10, 2023

View reviewed changes

Saransh-cpp reviewed Jul 11, 2023

View reviewed changes

Saransh-cpp reviewed Jul 13, 2023

View reviewed changes

src/scicookie/{{cookiecutter.project_slug}}/build-system/hatch-pyproject.toml Outdated Show resolved Hide resolved

Saransh-cpp requested changes Jul 14, 2023

View reviewed changes

ayeankit force-pushed the hat branch 2 times, most recently from 4eaea36 to 6cb7a4b Compare July 20, 2023 17:31

ayeankit and others added 20 commits July 21, 2023 00:10

Added hatchling as an option for build-system

2adad87

Update dev.yaml

7c8c006

Update guide.md

2593d14

Update post_gen_project.py

3d5fe4e

Update Makefile

2a09069

Update base.sh

7f00e1e

Rename hatch-pyproject.toml to hatchling-pyproject.toml

73d86c4

Update base.sh

a09eb2b

Update base.sh

06f797f

Update base.sh

25a5a44

Update base.sh

5b6dc25

Update docs/guide.md

ab3f9c8

Co-authored-by: Saransh Chopra <saransh0701@gmail.com>

Update hatchling-pyproject.toml

ec8742d

Update guide.md

eda53cc

Update base.sh

4595fbc

Update src/scicookie/cookiecutter.json

0839886

Co-authored-by: Saransh Chopra <saransh0701@gmail.com>

Update src/scicookie/{{cookiecutter.project_slug}}/Makefile

8f8dc67

Co-authored-by: Saransh Chopra <saransh0701@gmail.com>

Update src/scicookie/{{cookiecutter.project_slug}}/conda/dev.yaml

22744d9

Co-authored-by: Saransh Chopra <saransh0701@gmail.com>

Update docs/guide.md

0512db3

Co-authored-by: Saransh Chopra <saransh0701@gmail.com>

Update src/scicookie/{{cookiecutter.project_slug}}/conda/dev.yaml

65e2437

Co-authored-by: Saransh Chopra <saransh0701@gmail.com>

ayeankit and others added 20 commits July 21, 2023 00:10

Update src/scicookie/{{cookiecutter.project_slug}}/docs/contributing.md

b58b0e3

Co-authored-by: Saransh Chopra <saransh0701@gmail.com>

Update and rename hatchling-pyproject.toml to hatch-pyproject.toml an…

129a3a0

…d changed dev-dependence

Update hatch-pyproject.toml and added vcs to build-system

ccff1a9

Update base.sh and added command to install hatch

d7ef5b7

Update base.sh pip install hatch

e014a34

Update hatch-pyproject.toml

6f58ba6

Update base.sh

7319ac2

Update hatch-pyproject.toml

ec64e0a

Update src/scicookie/{{cookiecutter.project_slug}}/build-system/hatch…

209540c

…-pyproject.toml Co-authored-by: Saransh Chopra <saransh0701@gmail.com>

Update src/scicookie/{{cookiecutter.project_slug}}/build-system/hatch…

791f013

…-pyproject.toml Co-authored-by: Saransh Chopra <saransh0701@gmail.com>

Update src/scicookie/{{cookiecutter.project_slug}}/build-system/hatch…

ab359c0

…-pyproject.toml Co-authored-by: Saransh Chopra <saransh0701@gmail.com>

Update hatch-pyproject.toml

d433442

Update base.sh

bb57094

Update base.sh

bfc9ced

Update hatch-pyproject.toml

6e7c35c

Update hatch-pyproject.toml

dbea178

added dynamic version

746be94

added base.sh for hatch build system

28dbbfc

Merge branch 'main' into hat

41e3d1f

version set to cookiecutter's version

4f487ad

ayeankit force-pushed the hat branch from 6cb7a4b to 4f487ad Compare July 21, 2023 12:49

ayeankit added 2 commits July 21, 2023 18:29

Update base.sh and make build

2793a93

Update build-system.sh

1494be9

Saransh-cpp requested changes Jul 21, 2023

View reviewed changes

ayeankit and others added 2 commits July 21, 2023 21:30

Update src/scicookie/__init__.py

3dd27fa

Co-authored-by: Saransh Chopra <saransh0701@gmail.com>

Update tests/smoke/build-system.sh

457cb2f

Co-authored-by: Saransh Chopra <saransh0701@gmail.com>

xmnlab merged commit 32704a5 into osl-incubator:main Jul 21, 2023
11 checks passed

github-actions bot added the released label Jul 24, 2023

		@@ -0,0 +1,135 @@
		[build-system]
		requires = ["hatchling>=1.17.1", "hatch-vcs>=0.3.0"]

		elif command -v hatch &> /dev/null; then
		COMMAND_PREFIX="hatch run"

feat(template): Added hatchling as an option for build-system #144

feat(template): Added hatchling as an option for build-system #144

Conversation

ayeankit commented Jul 7, 2023 • edited Loading

Pull Request description

How to test these changes

Pull Request checklists

Additional information

Reviewer's checklist

Saransh-cpp left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ayeankit commented Jul 10, 2023

Saransh-cpp left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

xmnlab commented Jul 10, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Anavelyz Jul 17, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Saransh-cpp Jul 18, 2023 • edited Loading

Choose a reason for hiding this comment

Saransh-cpp left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Saransh-cpp left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ayeankit commented Jul 21, 2023

github-actions bot commented Jul 24, 2023

ayeankit commented Jul 7, 2023 •

edited

Loading

Anavelyz Jul 17, 2023 •

edited

Loading

Saransh-cpp Jul 18, 2023 •

edited

Loading