Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use weldx-widgets to reduce dependencies of weldx #749

Merged
merged 43 commits into from
May 12, 2022

Conversation

marscher
Copy link
Contributor

@marscher marscher commented May 2, 2022

Changes

Use weldx-widgets

  • removed k3d and mpl backends, types, colors, and tests.
  • removed warning about experimental package usage.

Related Issues

Closes #746

Checks

  • updated CHANGELOG.rst
  • updated tests

@codecov
Copy link

codecov bot commented May 2, 2022

Codecov Report

Merging #749 (5bbe914) into master (418cf3d) will increase coverage by 0.96%.
The diff coverage is 88.88%.

@@            Coverage Diff             @@
##           master     #749      +/-   ##
==========================================
+ Coverage   95.81%   96.78%   +0.96%     
==========================================
  Files          92       90       -2     
  Lines        6500     5999     -501     
==========================================
- Hits         6228     5806     -422     
+ Misses        272      193      -79     
Impacted Files Coverage Δ
weldx/util/util.py 94.65% <66.66%> (-2.12%) ⬇️
weldx/transformations/local_cs.py 96.34% <75.00%> (-0.37%) ⬇️
weldx/visualization/__init__.py 95.23% <94.73%> (+7.73%) ⬆️
weldx/core.py 93.71% <100.00%> (+0.02%) ⬆️
weldx/geometry.py 96.60% <100.00%> (+0.01%) ⬆️
weldx/measurement.py 95.60% <100.00%> (+0.02%) ⬆️
weldx/transformations/cs_manager.py 98.79% <100.00%> (+<0.01%) ⬆️
weldx/visualization/colors.py 100.00% <100.00%> (+10.90%) ⬆️
weldx/visualization/types.py 100.00% <100.00%> (ø)
weldx/welding/groove/iso_9692_1.py 99.52% <100.00%> (+<0.01%) ⬆️

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 418cf3d...5bbe914. Read the comment docs.

@github-actions
Copy link

github-actions bot commented May 2, 2022

Unit Test Results

       1 files  ±0         1 suites  ±0   2m 59s ⏱️ +19s
2 145 tests  - 1  2 145 ✔️  - 1  0 💤 ±0  0 ±0 

Results for commit 5bbe914. ± Comparison against base commit 418cf3d.

♻️ This comment has been updated with latest results.

@vhirtham
Copy link
Collaborator

vhirtham commented May 9, 2022

The title left me a bit confused since wxWidgets is a C++ GUI library 😄

@marscher marscher changed the title Use wx widgets Use weldx-widgets to reduce dependencies of weldx May 9, 2022
@marscher
Copy link
Contributor Author

@vhirtham We have discussed the mypy issue #752 and all linting issues left in PR are unrelated to the changes made. So in my opinion this is ready to be merged.

@marscher marscher requested a review from vhirtham May 11, 2022 12:42
@vhirtham
Copy link
Collaborator

@marscher yes, we should ignore the mypy issues for now. I will start a new PR soon. However, the other linter errors (deepsource) should be fixed. I had a look at the reported issue. If the behavior is intended, just place a corresponding comment to the line so that it is ignored by deepsource. It complains about an open call with an external reference.

@marscher
Copy link
Contributor Author

The code triggering this external open issue in _clean_notebook is a private function intended to clean notebooks, but I think this function is only a developer util, so it should be moved outside of the main package.

since this tool is only a developer utility.
Copy link
Collaborator

@vhirtham vhirtham left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@vhirtham
Copy link
Collaborator

@marscher You can just silence the warning with # skipcq: + error code. For example # skipcq: PYL-W0622

@marscher marscher merged commit d19acc1 into BAMWelDX:master May 12, 2022
@marscher marscher deleted the use-wx-widgets branch May 12, 2022 12:04
@CagtayFabry
Copy link
Member

cool !

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

move plotting features to weldx-widgets
3 participants