fixed the >>>cp: are the same file<<< error

[denizural]

As Laurent Oziel mentioned during the workshop today, FESOM compilation (esm_master comp-fesom-1.4) ends with an error like subprocess.CalledProcessError: Command '['cp', 'fesom-2.0/bin/fesom.x', 'fesom-2.0/bin']' returned non-zero exit status 1.

Although this is not a real error, subprocess.run() raises an exception since the shell return status ($?) is not 0. The reason is FESOM executables are hard linked and they share the same inode numbers. Therefore, copying these files on each other causes non-zero return.

I solved this issue by wrapping the subprocess.run() inside a try-except block and simply ignore the CalledProcessError exception.

Now the compilation completes without any errors but only the message:

cp: ‘fesom-1.4/bin/fesom’ and ‘fesom-1.4/bin/fesom’ are the same file

This can also be avoided (if you want) by adding 2>/dev/null to the copy command in task.py.

[denizural]

I made another commit after the discussion. I checked the compilation using FESOM 1.4.

Here are what I did

1) additon of pathlib

• I believe that pathlib module (Python 3.4, 2014) is a much better tool to use for constructing the directories. In the future we need to gradually refactor the code using that.
• Functions like resolve() or absolute(), parent are extremely useful and they remove the requirement for error-prone indexings. Eg. binfile.split("/")[-1]

2) More meaningful variable names for readability:

• I think ESM-Tools codes suffer from a good naming convention. clean_command looks like a string rather than a boolean command. Similarly path2bin_origin is the parent directory but it is hard to understand. Maybe binary_file_parent is better.

3) More "Pythonic" code:

• Constructions of the command strings span over multiple lines and often repeated (unnecessary duplication).
• I simply construct the command strings like: cmd_str = " ".join(["cp", str(binary_file_path), str(bin_path_target)])
• and simply append them to the command lists: Eg. command_list.append(cmd_str) real_command_list.append(cmd_str)
• So that the duplications are avoided and readability is better

[denizural]

I think the Pathlib stuff will need to be a general overhaul at some point. We have another big overhaul with the logging changes @seb-wahl wanted...

Only two small thing: I'd remove the line continuations (something we can automatically solve if we all agree to a code-formatter), and maybe the should_copy_files can be defined in one go.

Yes, but quite recently I switched to the PEP 8 standards and obey the Benevolent Dictator For Life: https://www.python.org/dev/peps/pep-0008/#maximum-line-length

[pgierz]

Here's an example of what I had in mind regarding code formatting: https://black.readthedocs.io/en/stable/index.html

[denizural]

Here's an example of what I had in mind regarding code formatting: https://black.readthedocs.io/en/stable/index.html

Black formatting looks really cool!

[pgierz]

@denizural, if you want, we can add it to automatically: here's how to put it into automatic testing for code style compliance. We could add that to all the esm sub packages.

https://github.com/esm-tools/esm_version_checker/blob/master/.github/workflows/python-package.yml

[mandresm]

I fixed an issue with recompilation of fesom. Now it should all work. I also passed again black to make sure my edits were well formatted.