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

feat: Add misc. repair and prepare tool methods #1293

Merged
merged 21 commits into from
Jul 31, 2024

Conversation

ansbdickens
Copy link
Contributor

@ansbdickens ansbdickens commented Jul 3, 2024

Description

Add PrepareTools and PrepareTools.share_topology method for list of bodies
Add ExtraEdgeProblemArea.fix() method
Add RepairTools.find_short_edges and ShortEdgeProblemArea

Issue linked

Exposes necessary components of a pyansys repair/prepare workflow

Checklist

  • I have tested my changes locally.
  • I have added necessary documentation or updated existing documentation.
  • I have followed the coding style guidelines of this project.
  • I have added appropriate unit tests.
  • I have reviewed my changes before submitting this pull request.
  • I have linked the issue or issues that are solved to the PR if any.
  • I have assigned this PR to myself.
  • I have added the minimum version decorator to any new backend method implemented.
  • I have made sure that the title of my PR follows Conventional commits style (e.g. feat: extrude circle to cylinder)

@ansbdickens ansbdickens requested a review from a team as a code owner July 3, 2024 15:45
@github-actions github-actions bot added the enhancement New features or code improvements label Jul 3, 2024
Copy link

codecov bot commented Jul 3, 2024

Codecov Report

Attention: Patch coverage is 76.08696% with 11 lines in your changes missing coverage. Please review.

Project coverage is 91.92%. Comparing base (c23b91e) to head (10c9b1e).

Files Patch % Lines
src/ansys/geometry/core/tools/problem_areas.py 65.38% 9 Missing ⚠️
src/ansys/geometry/core/tools/prepare_tools.py 90.90% 1 Missing ⚠️
src/ansys/geometry/core/tools/repair_tools.py 87.50% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1293      +/-   ##
==========================================
- Coverage   92.03%   91.92%   -0.11%     
==========================================
  Files          86       86              
  Lines        6713     6759      +46     
==========================================
+ Hits         6178     6213      +35     
- Misses        535      546      +11     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@ansbdickens ansbdickens self-assigned this Jul 3, 2024
src/ansys/geometry/core/tools/prepare_tools.py Outdated Show resolved Hide resolved
src/ansys/geometry/core/tools/prepare_tools.py Outdated Show resolved Hide resolved
src/ansys/geometry/core/tools/repair_tools.py Outdated Show resolved Hide resolved
Grammar, phrasing, and convention cleanup of comments

Co-authored-by: Kathy Pippert <84872299+PipKat@users.noreply.github.com>
@RobPasMue
Copy link
Member

RobPasMue commented Jul 16, 2024

I've been out on a trip until today. Let me work on this today morning - sorry for the delay. @umutsoysalansys brings a good point though. Tests have to be added.

@RobPasMue
Copy link
Member

@ansbdickens - please test my changes. They should work and simplify your PR

@umutsoysalansys umutsoysalansys marked this pull request as draft July 17, 2024 01:04
@umutsoysalansys umutsoysalansys self-assigned this Jul 17, 2024
@RobPasMue
Copy link
Member

@umutsoysalansys - I am reverting your changes. They broke lots of things in the process...

doc/changelog.d/1306.added.md Outdated Show resolved Hide resolved
tests/integration/test_prepare_tools.py Outdated Show resolved Hide resolved
@RobPasMue RobPasMue force-pushed the feat/bdickens/repair-prepare-workflow06 branch from 1a2d919 to c4f083c Compare July 17, 2024 06:09
@RobPasMue
Copy link
Member

HI @umutsoysalansys - something is wrong with this test:

def test_fix_extra_edge(modeler: Modeler):
    """Test to find and fix extra edge problem areas."""
    skip_if_linux(modeler, test_fix_extra_edge.__name__, "repair_tools")  # Skip test on Linux
    design = modeler.open_file(FILES_DIR / "ExtraEdgesDesignBefore.scdocx")
    problem_areas = modeler.repair_tools.find_extra_edges(design.bodies)
    assert problem_areas[0].fix().success is True

Assert is throwing back False is True which means that the problem_areas are not being fixed. Can you look into it?

@umutsoysalansys
Copy link
Contributor

@RobPasMue, I don't think there is anything wrong or different for this test and it is passing locally on Discovery but getting error on headless with this "Not fully implemented in Parasolid" remark.
image

@RobPasMue
Copy link
Member

As long as it doesn't pass on the Geometry Service @umutsoysalansys we cannot merge it. Could you solve this on the server implementation?

@umutsoysalansys
Copy link
Contributor

umutsoysalansys commented Jul 30, 2024

@RobPasMue yeah I can look into it, meanwhile you can stash that piece of the PR to somewhere else and merge the working parts?

@RobPasMue
Copy link
Member

Sure, we can do that - I will completely skip the test. I will open a GH issue and assign it to you to solve this problem

@RobPasMue RobPasMue marked this pull request as ready for review July 31, 2024 06:05
@RobPasMue RobPasMue enabled auto-merge (squash) July 31, 2024 06:28
@RobPasMue RobPasMue merged commit 896829b into main Jul 31, 2024
46 of 59 checks passed
@RobPasMue RobPasMue deleted the feat/bdickens/repair-prepare-workflow06 branch July 31, 2024 06:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New features or code improvements
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants