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

Add from_file and write_to_file to SpatialData #265

Merged
merged 7 commits into from
Mar 11, 2021

Conversation

vhirtham
Copy link
Collaborator

@vhirtham vhirtham commented Mar 10, 2021

Changes

  • add meshio as new dependency
  • add from_file and write_to_file to SpatialData

Checks

  • updated CHANGELOG.md
  • updated tests
  • updated doc/
  • update example/tutorial notebooks

@codecov
Copy link

codecov bot commented Mar 10, 2021

Codecov Report

Merging #265 (551e104) into master (2bda4dd) will increase coverage by 0.00%.
The diff coverage is 100.00%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #265   +/-   ##
=======================================
  Coverage   99.65%   99.65%           
=======================================
  Files          75       75           
  Lines        4009     4019   +10     
=======================================
+ Hits         3995     4005   +10     
  Misses         14       14           
Impacted Files Coverage Δ
weldx/geometry.py 99.39% <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 175b227...551e104. Read the comment docs.

@@ -2328,6 +2329,26 @@ def __post_init__(self):
if not self.triangles.ndim == 2:
raise ValueError("SpatialData triangulation must be a 2d array")

@staticmethod
def from_file(file_name: str) -> "SpatialData":
Copy link
Member

Choose a reason for hiding this comment

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

pathlib.Path also seems to be supported by meshio so we might add that to the type hints

Copy link
Collaborator Author

@vhirtham vhirtham Mar 10, 2021

Choose a reason for hiding this comment

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

yes. I am currently experimenting with it and writing the test ;)

Copy link
Member

Choose a reason for hiding this comment

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

I don't think a test is necessary here actually since this is such a thin wrapping, I would be fine with #noqa as well 😉

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

It's already there and it actually found a bug in the write_to_file :p

Copy link
Contributor

@marscher marscher Mar 10, 2021

Choose a reason for hiding this comment

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

It looks like the library is leaking file handles. But by using a sane OS this ain't a problem :p

@CagtayFabry
Copy link
Member

@vhirtham I stumbled upon this nice utility in meshio that might come in handy for other file operations later, just dropping it here :)

@vhirtham vhirtham marked this pull request as ready for review March 10, 2021 12:43
Copy link
Contributor

@marscher marscher left a comment

Choose a reason for hiding this comment

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

LGTM!

@marscher
Copy link
Contributor

@vhirtham I stumbled upon this nice utility in meshio that might come in handy for other file operations later, just dropping it here :)

This is actually meant to be used when using their readers/writers to avoid leaking file handles!

@pep8speaks
Copy link

Hello @vhirtham! Thanks for updating this PR.

Line 6:13: E261 at least two spaces before inline comment
Line 7:31: E261 at least two spaces before inline comment
Line 8:16: E261 at least two spaces before inline comment
Line 11:67: E202 whitespace before ')'
Line 13:1: E128 continuation line under-indented for visual indent
Line 15:23: E999 SyntaxError: invalid syntax
Line 16:5: E113 unexpected indentation
Line 17:9: E128 continuation line under-indented for visual indent
Line 18:9: E128 continuation line under-indented for visual indent
Line 27:1: E302 expected 2 blank lines, found 1
Line 47:89: E501 line too long (96 > 88 characters)
Line 56:1: E302 expected 2 blank lines, found 1
Line 63:89: E501 line too long (105 > 88 characters)
Line 66:24: E201 whitespace after '{'
Line 68:89: E501 line too long (97 > 88 characters)
Line 84:89: E501 line too long (98 > 88 characters)
Line 87:1: E302 expected 2 blank lines, found 1
Line 102:89: E501 line too long (96 > 88 characters)
Line 110:1: E302 expected 2 blank lines, found 1
Line 118:89: E501 line too long (120 > 88 characters)
Line 120:89: E501 line too long (113 > 88 characters)
Line 128:89: E501 line too long (103 > 88 characters)
Line 130:89: E501 line too long (96 > 88 characters)
Line 132:89: E501 line too long (103 > 88 characters)
Line 134:89: E501 line too long (96 > 88 characters)
Line 136:89: E501 line too long (97 > 88 characters)
Line 140:89: E501 line too long (107 > 88 characters)
Line 153:1: E305 expected 2 blank lines after class or function definition, found 1
Line 156:18: E261 at least two spaces before inline comment

@vhirtham vhirtham merged commit 23bdacf into BAMWelDX:master Mar 11, 2021
@vhirtham vhirtham deleted the meshio_new branch March 11, 2021 08:04
@CagtayFabry CagtayFabry linked an issue Mar 11, 2021 that may be closed by this pull request
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.

3D geometry/mesh representation
4 participants