Renamed files, pre-commit flake8

[ewanchamberlain]

Renamed:

• cobaya/typing.py -> cobaya/cobaya_typing.py, cobaya/yaml.py -> cobaya/cobaya_yaml.py to prevent circular imports.
• cobaya/input.py -> cobaya/cobaya_input.py to prevent clashes with input().

Added .flake8 with options from the docs.
Added pre-commit file to run flake8 on commit.

[cmbant]

Thanks for this. Do you have a clean test case that will reproduce circular import error and know if there is also a problem with the current public (pypi) release? The only times I've seen it in the past are in meta-loading contexts like using pycharm debugger, where it's not clear where the problem lies.

The potential problem with renaming the modules is that there are now multiple external likelihoods inheriting and using Cobaya, so if you make a breaking name change like this they are likely to break (and there's no very obvious way to make a compatibility layer that wouldn't also potentially trigger circular dependency).

[JesusTorrado]

Hi @ewanchamberlain

Thanks for the patch! A couple of comments:

I agree naming modules as built-in ones (or commonly used packages, as for yaml) is not good practice. But first, as @cmbant says, let's make sure that this is really problematic and has a workable solution.

In any case, let's just think of names that are less redundant than cobaya_X. I propose e.g. input->input_processing, typing->typing_conventions, yaml->yaml_helpers.

On the other hands, wrappers could (I think) be dealt with at the __init__ level, instead of having an empty file, right? If that's the case, I'd rather take that approach. But as @cmbant mentioned, maybe that's also problematic? Not sure at the moment.

For flake8, I would strongly prefer that we add pyproject-flake8 to the test requisites and add the flake8 config as a pyproject.toml section (see the readme in the PyPI page of that package).

[ewanchamberlain]

Hi @JesusTorrado,

As far as I can tell this error only occurs when using any gh version, while the PyPI version works fine. Still, better to fix the error now than later down the line.

I'm easy on names, I'll change them to your suggestion unless @cmbant has other suggestions.

Wrapper modules are easier to read and I'm not sure putting them in __init__.py will allow a deprecation warning to be sent. Regardless, either method will need double checking to make sure that no ImportErrors arise.

I can add pyprojects-flake8, though probably will need to keep flake8 for the pre-commit.

[cmbant]

The python rules do seem to be quite clear that "import typing" always means the "typing" available from the global path, not the local package. I would have thought "pip install -e" shouldn't add the inside of the cobaya/ directory to path, in which case cobaya.typing should not be on the path and hence still OK, so a bit odd (and certainly not something I see generally, though have occasionally seen similar when e.g. debugging in PyCharm where things are being called indirectly via pytest/internal debugging wrappers that somehow get module imports messed up).

importing yaml_load from cobaya.yaml seems to be quite common in 3rd party repos, so should keep that working. Does cobaya.yaml actually cause a problem?

[ewanchamberlain]

Are you sure it's not saying that "import typing should only be used to refer to the standard library"? Running cobaya/typing.py on the master branch will cause the circular import error. The error has even been updated in Python3.13 to recommend that the file be renamed to stop clashes with the standard library typing module. Almost certainly this is what's happening when you're debugging, and when pytest is run you get errors stemming from cobaya/typing.py only being partially imported.

cobaya/yaml.py also causes a circular import error. Most likely packages using this are using the PyPI/conda version of cobaya, which as I've said before work fine, but this is more out of luck than anything.

[cmbant]

The current code never does "import typing" not referring to the standard module, so doesn't seem inconsistent.

Renaming certainly avoids the issue, though "running" cobaya/typing.py etc I think is not something we need to work, and it does still seem to me that what we have should work (I think the Pycharm issue disappeared after some update, suggesting a temporary module bug in one of the packaging/testing libraries).

numpy of course also has a numpy.typing submodule, which is probably why we used that name.

[ewanchamberlain]

That is not the issue. The issue is that there is a module named typing that wants to import from typing. This is a circular import error. The interpreter incorrectly tries to import cobaya/typing.py from within cobaya/typing.py.

Modules should not give uncaught errors when run, and certainly not when imported. The current project in theory should not work because of the above error.

The numpy typing module is structured differently, being a directory rather than a file, and does not import from the standard typing library.

[ewanchamberlain]

@JesusTorrado I've renamed the files to your suggested names and added pyproject-flake8.

[JesusTorrado]

At the end of the day, I think I agree with @cmbant. If at all, the only would-be-problem is typing.py, and you are not supposed to import internal package modules by themselves: if the package is not installed, all the internal [package].[module] within the imported module will fail regardless of circular dependencies, and if the package is installed, you are supposed to import modules from the package, not directly.

If a linter/debugger/LSP-server complains, it's a bug of its name resolver.

I would personally close this and open a separate one for the flake8 thing.

[JesusTorrado]

Hi @ewanchamberlain

Are you intending to create another PR for the pre-commit hook with pyproject.toml?

There is no particular hurry, but if you have decided not to, please let me know to try to experiment with it myself. But it would be better if you appear as the one doing the commits :)

[ewanchamberlain]

Hi @JesusTorrado
Sorry, somehow missed your last comment. I'll try to get the PR done sometime next week.