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

Adding a little code coverage to key Assembly methods #1670

Merged
merged 4 commits into from
Apr 5, 2024

Conversation

john-science
Copy link
Member

@john-science john-science commented Mar 26, 2024

What is the change?

Here I add a little code coverage to three Assembly methods:

  • Assembly.add()
  • Assembly.getArea()
  • Assembly.getDim()

Also, Assembly.add() had a couple lines of broken code that I removed.

Why is the change being made?

This was a request. And these are important methods.

Extra Note

I would just like to document somewhere that I think the Assembly.getArea() method is properly insane. When an Assembly has no children/Blocks, this method returns a "default area" of 1.0. Surely that should be an error? Or it should return None. Or it should return 0.0. Returning a valid numerical value just seems crazy.

Thoughts?


Checklist

  • The release notes have been updated if necessary.
  • The documentation is still up-to-date in the doc folder.
  • The dependencies are still up-to-date in pyproject.toml.

@john-science john-science added the testing Related to tests label Mar 26, 2024
cur = self.assembly.getArea()
ref = math.sqrt(3) / 2.0 * self.hexDims["op"] ** 2
places = 6
self.assertAlmostEqual(cur, ref, places=places)
self.assertAlmostEqual(cur, ref, places=6)
Copy link
Member

Choose a reason for hiding this comment

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

the precision of this can be increased. in my testing you can even swap it with a self.assertEqual(cur, ref)

@albeanth
Copy link
Member

When an Assembly has no children/Blocks, this method returns a "default area" of 1.0. Surely that should be an error? Or it should return None. Or it should return 0.0. Returning a valid numerical value just seems crazy.

Yeah I agree. It's odd. If you make the change, do a lot of other things break?

@ntouran @jakehader any historical significance here?

@john-science john-science merged commit 6222242 into main Apr 5, 2024
15 checks passed
@john-science john-science deleted the assembly_tests branch April 5, 2024 21:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
testing Related to tests
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants