Prettier formatter

[Domejko]

Closes #2004

[Domejko]

We'll also need to mention prettier in docs/user/contributing.rst and add CI in .github/workflows/style.yaml

In docs/user/contributing.rst I assume that you would like to just mention that Prettier is used in CI to keep the files format consistent. As it comes to .github/workflows/style.yaml it have been already added to CI but due to the size and number of changes in this PR you could missed it. Let me know are you satisfied with this implementation in CI or does some changes have to be made.

[adamjstewart]

In docs/user/contributing.rst I assume that you would like to just mention that Prettier is used in CI to keep the files format consistent.

And also how to use prettier yourself. Basically, add it to the "Linters" section and follow a similar structure as ruff/mypy.

[adamjstewart]

We should also add a pre-commit hook: https://prettier.io/docs/en/precommit#option-2-pre-commithttpsgithubcompre-commitpre-commit

[adamjstewart]

Current test failure was fixed in main, if you update your branch it should pass.

[Domejko]

Yes it already have passed once in Reverted changes in data folder but I have used git add . and my .idea folder have been added so I've added .idea to .gitignore removed .idea form PR and next test have failed.

My branch says that is updated "This branch is 30 commits ahead of microsoft/torchgeo:main."

[adamjstewart]

Does your branch include https://github.com/microsoft/torchgeo/pull/2024? That should prevent CI from installing pytest from the cache on macOS.

[Domejko]

No it doesn't. I will update it now.

[Domejko]

That's strange, when I was testing this on my fork I got

Run npx prettier . --check
Checking formatting...
All matched files use Prettier code style!

and here again it can't find it and need to install it.

[adamjstewart]

What if you run it as prettier instead of npx prettier? Does that help?

[Domejko]

Unfortunately it have to be run through npx. The issue is that npm ci need to be run in directory where JSON package files are and there is no flag to provide nested path to them. So when we run it in requirements/ it install node_modules there and as we run check command from root it doesn't see modules there. Strange is this that it worked at first on my fork.

We might try to use npm ci with --prefix flag and absolute path to project root because when I tried with simple ../ it didn't work. But I think that using npm install instead with path to requirements/ will be the cleanest solution. Installation process with npm ci takes 393ms so even if it doubles it will be unnoticeable.

[adamjstewart]

I guess in order of priorities, I have:

1. It MUST install a stable version from a lock file, it CAN'T just install the latest version
2. I would prefer that we store these files in requirements if we can, but we could put them in the root if that isn't possible
3. I would prefer that we don't install the same thing in CI twice

We could change the working-directory for every step of the pipeline so that installation and running happen in requirements and we pass it .. to run on the whole project. Kinda ugly, but probably works.

[Domejko]

I have used and tested npm install requirements/ and everything works fine. I had to add to package.json "name": "torchgeo" since without it git wasn't happy and we could not clone prettier. It looks like this:

      - name: Installing prettier
        run: |
          npm install requirements/
          npm cache clean --force
      - name: List npm dependencies
        run: npm ls --all
      - name: Run prettier formatting
        run: npx prettier . --check

Wanted to ask also are you sure you don't want to put node_modules to .gitignore ? Because when contributors will start to use it not all of them will have it set up as global ignore. It's not IDE specific.

[adamjstewart]

node_modules should be in .gitignore, IDE-specific things should not.

[Domejko]

My pleasure, I'm happy that I could contribute to this project and too me it was also a very good experience, I have learn new tools that for sure might come in handy in the future.