Enable Code Formatting Checks

[cssherman]

No description provided.

[cssherman]

@TotoGaz - I'm turning on the yapf code-formatting checks here, and the results for various mesh doctor files look good. A couple of notes:

1. In one location, I found that implicit tuple assignment could cause problems with the formatter (e.g. some_tuple = 1, 2). Changing this to an explicit assignment worked (e.g. some_tuple = (1, 2)).
2. I added a couple of yapf enable/disable statements to one of the mesh doctor tests that included some structured mesh data.

[TotoGaz]

What about using https://github.com/google/yapf?tab=readme-ov-file#space_inside_brackets ?
I find it more readable + it's like in geos.

[TotoGaz]

Suggested change

	spaces_before_comment = 4
	spaces_before_comment = 2

maybe it's enough?

[cssherman]

Trying out the space_inside_brackets setting, I agree that the resulting code looks good. I'm happy to move forward with it.

[untereiner]

Hello! The new trend is to use ruff (written in Rust): 50x faster than black and 100x than yapf. Also, would it be possible to migrate to poetry ? (and use pyproject.toml for configurations)

[TotoGaz]

Hello! The new trend is to use ruff (written in Rust): 50x faster than black and 100x than yapf. Also, would it be possible to migrate to poetry ? (and use pyproject.toml for configurations)

Does it have as many configuration options?
Is it easy to deploy? (since it's in rust, maybe it's more complicated to deploy?)

[untereiner]

its rust wrapped to python to be fast (as opposed to pure python :) ) I don't know about the options. It produces pep8 compatible code.

yapf	ruff
spaces_before_comment	too-few-spaces-before-inline-comment
split_before_logical_operator=true	PEP8 default
column_limit	max-line-length

To deploy:
pip add ruff
or
poetry add ruff

[cssherman]

I'm not opposed to using another formatter (although, I'm also not terribly worried about performance because yapf already does the job in a couple of seconds). If there are other quality of life improvements that we can leverage, that would be great. @untereiner - Have you used any of the available linter plugins for ruff?

I don't have any problems with poetry, since it is back-compatible with pip/conda (which most of our users will probably use).

[untereiner]

I was proposing this change because this style is based on PEP8 (not google or facebook). So not sure yapf has an added value. I was using flake8, isort and black. I am in the process of changing to ruff my projects. So just one tool for linting, auto-formating, include sorting) using a modern build system interface (pyproject.toml)

[TotoGaz]

Is the installation easy on Pangea3 for ex? There should be binaries for ppc, but I think we should check first if all our targets are properly supported.

[untereiner]

very good point @TotoGaz
@sframba do you know or can test if a pip install poetry ruff works on pangea3 ?

[sframba]

@untereiner the packages seem to be available for ppc, I am trying to install them on Pangea3, I'll keep you posted

[cssherman]

@untereiner @TotoGaz - I've played around with ruff and its linter plugins for vscode/sublime, and I'm happy with it. Assuming that plays well with Pangea3, I'm OK moving forward with it.

[sframba]

@untereiner the packages seem to be available for ppc, I am trying to install them on Pangea3, I'll keep you posted

It seems that the python packages are available for ppc. We don't have any Rust distribution that I know of though. But we can ask the maintenance team to deploy one, or at the very least deploy a local version via Jenkins ourselves

[untereiner]

I think you do not need any rust distribution installed locally. The code is compiled to the target system without the need of any dependencies.
The documentation says: pip install ruff
Digging a bit more: They have ppc64 in their CI and releases

You should be able to make on Pange3 :

pip install ruff
ruff -h

[TotoGaz]

I think you do not need any rust distribution installed locally. The code is compiled to the target system without the need of any dependencies. The documentation says: pip install ruff Digging a bit more: They have ppc64 in their CI and releases

You should be able to make on Pange3 :

pip install ruff
ruff -h

On their websites, they say there are binaries for ppc. I simply want to be sure they're working for Lassen and Pangea3.
If we ever need to install anything aside to let pip perform the compilation (which may fail for whatever reason) on the fly, then we better stick to a pure python tool that will be smooth to use for everyone. Our python codebase is tiny, we need no perf there.

[sframba]

Unfortunately you cannot pip install on Pangea3 directly from the network, the installation nodes have no internet connection. You have to manually download the packages (and dependencies as needed) on the login node and then install them from the given dir.

The Rust compiler is required by cryptography, which in turn is required by poetry. I am currently trying with version cryptography==3.3.2 which shouldn't need Rust. I'll keep you posted

[sframba]

I can confirm that cryptography==3.3.2 does not require Rust, so poetry seems to be ok. However we have a conflict on Cython, since ruff requires Cython~=3.0.0 while our numpy/contourpy versions require Cython<3.0.0. I'll look into it

[sframba]

Compilation is ok on Pangea3 with the following versions. Are they acceptable for you?

cryptography==3.3.2
setuptools-rust>=1.7.0
poetry
ruff
Cython==0.29.36

Also,

$ ruff -h
<jemalloc>: Unsupported system page size
<jemalloc>: Unsupported system page size
memory allocation of 5 bytes failed
Aborted (core dumped)

I suspect this might be an issue. Is there something you would like to test on Pangea3 in particular? So I can focus my debugging

[untereiner]

@sframba thank you. This quite extensive installation process demonstrates that ruff isn't adapted to Pangea3. @TotoGaz I was too optimistic.

[sframba]

@untereiner let me try to debug the issue on Pangea3 first, it would be a pity to give up just for this. Can someone do the same tests on Lassen?

[TotoGaz]

@sframba thank you. This quite extensive installation process demonstrates that ruff isn't adapted to Pangea3. @TotoGaz I was too optimistic.

Actually, I was quite convince we were going to face an issue, but still it was forth testing first 😉 🤣

@untereiner let me try to debug the issue on Pangea3 first, it would be a pity to give up just for this. Can someone do the same tests on Lassen?

@sframba you can do as you want, but TBH I wouldn't waste time on it. We have very good replacement tools. W.r.t. how "critical" this task is in the geos ecosystem, I would let it go ❄️

[cssherman]

@TotoGaz @untereiner @sframba - let's continue with merging these changes, and plan to re-visit this