run PEP585 upgrade

[CagtayFabry]

Changes

• upgrade type hints to build in types (using PEP585 Upgrade)

I have disabled the pre-commit hook since it breaks some of our type definitions (it might work but I didn't check further)

Using the pep585 type hints should be the new default with from __future__ import annotations

[codecov]

Codecov Report

Merging #678 (4cb0a10) into master (045a054) will increase coverage by 0.00%.
The diff coverage is 100.00%.

@@           Coverage Diff           @@
##           master     #678   +/-   ##
=======================================
  Coverage   95.89%   95.90%           
=======================================
  Files          92       92           
  Lines        6310     6321   +11     
=======================================
+ Hits         6051     6062   +11     
  Misses        259      259           

Impacted Files	Coverage Δ
weldx/asdf/cli/welding_schema.py	97.00% <100.00%> (+0.03%)	⬆️
weldx/asdf/extension.py	100.00% <100.00%> (ø)
weldx/asdf/file.py	97.01% <100.00%> (+0.01%)	⬆️
weldx/asdf/types.py	93.84% <100.00%> (+0.09%)	⬆️
weldx/asdf/util.py	90.20% <100.00%> (+0.06%)	⬆️
weldx/asdf/validators.py	98.12% <100.00%> (+0.03%)	⬆️
weldx/config.py	82.66% <100.00%> (+0.23%)	⬆️
weldx/core.py	93.07% <100.00%> (ø)
weldx/geometry.py	97.49% <100.00%> (-0.01%)	⬇️
weldx/measurement.py	95.57% <100.00%> (ø)
... and 20 more

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 045a054...4cb0a10. Read the comment docs.

[github-actions]

Unit Test Results

       1 files  ±0         1 suites  ±0   2m 6s ⏱️ -15s
2 101 tests ±0  2 101 ✔️ ±0  0 💤 ±0  0 ❌ ±0 

Results for commit 4cb0a10. ± Comparison against base commit 045a054.

♻️ This comment has been updated with latest results.

[marscher]

What is the purpose of using more specific types, e.g. tuple vs. typing.Tuple, in the annotation? Eventually, we do not want to restrict it to be specific to that particular type, but a broader common interface. Of course this can be changed if desired, I just wonder what this has to do with pep585.

[CagtayFabry]

What is the purpose of using more specific types, e.g. tuple vs. typing.Tuple, in the annotation? Eventually, we do not want to restrict it to be specific to that particular type, but a broader common interface. Of course this can be changed if desired, I just wonder what this has to do with pep585.

this doesn't really change anything besides that after pep585 it is allowed to use tuple[] instead of Tuple[] which simply makes it easier (or more 'natural') to type annotations

[vhirtham]

So most of the time it is just replacing lower-case with upper-case in the type hints and a different import source. Well, to me it is not really important which annotation we use, as long as everything works and it is consistent.

However:

I have disabled the pre-commit hook since it breaks some of our type definitions (it might work but I didn't check further)

So can we fix this?

[marscher]

Now I've learned that the type duplication of collection types in the typing module is now deprecated. Thanks for fixing this.
Why is the pre-commit hook disabled? Are there more spots in the code, where we still use the typing collections?

[CagtayFabry]

However:

I have disabled the pre-commit hook since it breaks some of our type definitions (it might work but I didn't check further)

So can we fix this?

I didn't want to spend the time digging into it, feel free to run the hook again and see the changes/errors (they are only a few)

Why is the pre-commit hook disabled? Are there more spots in the code, where we still use the typing collections?

See above, in addition the hook does 'mess' with the imports (adding new ones, removing some) but doesn't respect isort or simliar. So sometimes pre-commit has to run twice (also the reason why I put early in the hook list)

This was mostly intended as a one time upgrade to switch to pep585 annotations, if we simply stick to that from now on we probably don't need to have the hook activated (or we can run it from time to time manually)

[marscher]

See above, in addition the hook does 'mess' with the imports (adding new ones, removing some) but doesn't respect isort or simliar. So sometimes pre-commit has to run twice (also the reason why I put early in the hook list)

The readme of the hook does mention that and recommends to run this hook in a "tandem" with isort. So I guess we need to figure how to do that.

quote:

Note: even though we remove and add imports reasonably well, I would recommend running this in tandem with hooks like isort to aggregate and sort your imports, and flake8 to discover any unused imports neither were able to remove.

[CagtayFabry]

The readme of the hook does mention that and recommends to run this hook in a "tandem" with isort. So I guess we need to figure how to do that.

exactly, if you simply run the hook before isort it will 'heal' itself (but the first commit attempt will still fail)
right now the hook doesn't finish cleanly so if you want to enable it someone would quickly have to look into the fixes necessary (or disable the hook for specific files) @vhirtham @marscher

[marscher]

RTD has now the following warning: weldx/geometry.py:docstring of weldx.geometry.SpatialData.time:: WARNING: more than one target found for cross-reference 'Time': weldx.Time, weldx.time.Time

This is surprising, since I only changed something in weldx/asdf/util.py

Eventually the new Sphinx version is being used now.

[CagtayFabry]

RTD has now the following warning: weldx/geometry.py:docstring of weldx.geometry.SpatialData.time:: WARNING: more than one target found for cross-reference 'Time': weldx.Time, weldx.time.Time

This is surprising, since I only changed something in weldx/asdf/util.py

Eventually the new Sphinx version is being used now.

to narrow these things down and check if a failed action passes on master you can always manually trigger a build for any branch from the actions page:
https://github.com/BAMWelDX/weldx/actions/workflows/docs.yml

I have started an additional build on master here: https://github.com/BAMWelDX/weldx/actions/runs/1718748392

[marscher]

Thanks. The default branch triggers the same warning. So we can either merge now and fix the warning in a separate PR or resolve it here. From my experience you prefer to keep things separated, right?

[vhirtham]

Thanks. The default branch triggers the same warning. So we can either merge now and fix the warning in a separate PR or resolve it here. From my experience you prefer to keep things separated, right?

It's the same on the tutorial PR. I will look into the problem next week.

[vhirtham]

Thanks. The default branch triggers the same warning. So we can either merge now and fix the warning in a separate PR or resolve it here. From my experience you prefer to keep things separated, right?

It's the same on the tutorial PR. I will look into the problem next week.

See #684