Skip to content

Conversation

boltma
Copy link

@boltma boltma commented Sep 29, 2025

Description

Add project_sheet method to modeler.

Issue linked

#5360

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 tests (unit, integration, system).
  • I have reviewed my changes before submitting this pull request.
  • I have linked the issue or issues that are solved by the PR if any.
  • I have agreed with the Contributor License Agreement (CLA).

@boltma boltma requested a review from a team as a code owner September 29, 2025 02:23
@ansys-cla-bot
Copy link

ansys-cla-bot bot commented Sep 29, 2025

All Contributor License Agreement (CLA) signatures have been captured successfully. Thanks for contributing!

Copy link
Collaborator

@SMoraisAnsys SMoraisAnsys left a comment

Choose a reason for hiding this comment

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

Hey @boltma, thanks a lot for this great contribution !
I'll have a look to test the code locally today, I left some comments in the mean time.

],
)
unclassified_new = [i for i in self.unclassified_objects if i not in unclassified]
if unclassified_new: # pragma: no cover
Copy link
Collaborator

Choose a reason for hiding this comment

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

Do you think that you could remove that pragma: no cover and add a unit test (using mocking / monkeypatch) to cover that scenario ?

Copy link
Author

Choose a reason for hiding this comment

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

Thanks for the feedback. I’ve updated the test, though I’m not sure I have followed the best practice.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I was thinking about adding a test inside of tests/unit, not tests/system but thanks for trying that :)
Since you are not part of the ansys organization on GitHub we can't use your PR to run the CI which runs on self-hosted runners. I'll cherry-pick your changes and create a new PR so that you are still the author of those changes and we can make it run the CI. If that's fine with you, I can also see how to add the mentionned tests/unit changes. Would that be fine for you ?

Copy link
Author

Choose a reason for hiding this comment

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

Sure, thank you so much!

Copy link
Collaborator

Choose a reason for hiding this comment

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

Sry for the delay, I'll have a look in the PR next week. This week was super busy. Thanks again for the contribution and your patience.

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.

2 participants