chore: convert to using relative imports

[JohnVillalovos]

First commit renames gitlab/__version__.py to gitlab.version.py to minimize confusion.

Switch to using relative imports to ensure that we are importing from
within our library.

Also use the form:
from foo import bar as bar
When we want to signify that we want the import to be re-exported.

https://mypy.readthedocs.io/en/stable/command_line.html#cmdoption-mypy-no-implicit-reexport

[codecov-commenter]

Codecov Report

Merging #1728 (6b3e7c0) into main (c6d7e9a) will increase coverage by 0.00%.
The diff coverage is 100.00%.

@@           Coverage Diff           @@
##             main    #1728   +/-   ##
=======================================
  Coverage   92.02%   92.03%           
=======================================
  Files          76       76           
  Lines        4790     4793    +3     
=======================================
+ Hits         4408     4411    +3     
  Misses        382      382           

Flag	Coverage Δ
cli_func_v4	81.38% <96.66%> (+0.01%)	⬆️
py_func_v4	80.24% <86.29%> (+0.03%)	⬆️
unit	83.18% <93.70%> (-0.02%)	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
gitlab/version.py	100.00% <ø> (ø)
gitlab/__init__.py	100.00% <100.00%> (ø)
gitlab/__main__.py	100.00% <100.00%> (ø)
gitlab/base.py	90.22% <100.00%> (ø)
gitlab/cli.py	98.40% <100.00%> (+0.03%)	⬆️
gitlab/client.py	90.51% <100.00%> (-0.05%)	⬇️
gitlab/config.py	94.44% <100.00%> (ø)
gitlab/const.py	100.00% <100.00%> (ø)
gitlab/mixins.py	91.52% <100.00%> (-0.03%)	⬇️
gitlab/v4/cli.py	81.25% <100.00%> (-0.14%)	⬇️
... and 61 more

[nejch]

Thanks @JohnVillalovos this is huge so I just had a few general comments first, the rest LGTM but would probably be affected by the other comments if we decide on this.

[nejch]

As above, should we maybe import only the stuff we need directly like with the other imports here in this commit?

Suggested change

	from . import config as gl_config
	from .config import GitlabConfigParser, ConfigError

[JohnVillalovos]

Yeah, can do that. I'm usually not a big fan of doing from foo import spam, egg where spam and egg are things inside the module. As then in my opinion it is harder when reading the code to know where something comes from. In this case when reading the code people can see it used as: gl_config.GitlabConfigParser. But then again I am also doing a from . import config as gl_config which is also somewhat confusing 😟

[nejch]

We seem to mix from-importing modules and functions/classes a lot (not a criticism of this PR, just how the project grew I assume). Maybe we could have a convention where we always functions/class if it's a reasonable number of things to import? E.g. if we don't need a large multi-line import, just import directly, otherwise the module.

Suggested change

	from . import cli
	from .cli import main

[JohnVillalovos]

My convention would lean towards just import the module, not items within the module. As I think it makes the code more readable. We read the code much much more then we ever write it.

My goal (not that I achieve it) is that we make the code is easy to read as possible to reduce "cognitive load". I say this as a vim user 😊

[nejch]

If we do the above, then:

Suggested change

	cli.main()
	main()

[JohnVillalovos]

Note: repeat of above comment

My convention would lean towards just import the module, not items within the module. As I think it makes the code more readable. We read the code much much more then we ever write it.

My goal (not that I achieve it) is that we make the code is easy to read as possible to reduce "cognitive load". I say this as a vim user 😊

[nejch]

For me that readability comes with full namespaces (I guess that has its own issues which is why you added relative imports) but not so much with relative imports.

For example with the change below, it's also not clear in the codebase whether we are calling functions from gitlab.cli or gitlab.v4.cli:

+   from .v4 import cli

# ...

    parser = _get_base_parser()
-    return gitlab.v4.cli.extend_parser(parser)
+    return cli.extend_parser(parser)

Maybe we can poke around in some popular packages and see what they are doing :) I'll have a look, feel free to add more here as well.

[nejch]

I checked a few and seems to me importing classes/functions is pretty common? black/requests/click/httpx all seem to almost exclusively do relative imports with functions/classes. mypy is more of a mixed bag and they do both, but not sure how they make the distinction.

[JohnVillalovos]

For example with the change below, it's also not clear in the codebase whether we are calling functions from gitlab.cli or gitlab.v4.cli:

+   from .v4 import cli

# ...

    parser = _get_base_parser()
-    return gitlab.v4.cli.extend_parser(parser)
+    return cli.extend_parser(parser)

Well I disagree not being clear (in this case) because they are within three lines of each other 😊 Of course they won't always be within three lines of each other. Unfortunately it is not valid syntax to do from . import v4.cli. To enhance clarity could do from .v4 import cli as v4_cli ??

But I personally find it much easier to comprehend cli.main() vs main() when reading the code. As it makes me think, "oh we are probably calling main() in a different module". Where with just main() I think, "we are calling the main() function in this module"

[JohnVillalovos]

Note 834677e is only a rebase with no functional changes. Had conflicts after another PR merged. Will work on fixing things in regards to comments later on today most likely.