xcfem / xc

finite element analysis package for civil engineering structures.
http://www.xcengineering.xyz/html_files/software.html
GNU General Public License v3.0
266 stars 54 forks source link

XC TEST: adding helper function for gmsh test #77

Closed ebrahimraeyat closed 3 years ago

ebrahimraeyat commented 3 years ago
lcpt commented 3 years ago

Hi, good work!

I didn't know about type hints. Do you know if there is some way to make that work in Python 2? If needed, we can abandon Python 2 but if there is something like from __future__ import hint we can continue supporting Python 2 a bit longer (yeah, I know it's already a year from the Python 2 close down :) ). I've googled a little about it, but I haven't found an easy way to make it Python 2 compliant.

Anyway this type hint thing is really useful, so I want to keep it.

Thanks a lot.

ebrahimraeyat commented 3 years ago

Hi @lcpt . I searched about it and found this solution, but I didn't compile the XC with python2, the example:

def getHoleAsPolygonalSurface(
    bolt_fastener: ASTM_materials.BoltFastener,
    surfaces: xc.SurfaceMap,
    points: xc.PntMap
    ) -> xc.PolygonalFace:
    # type: (...) -> xc.PolygonalFace
    ''' Create the polygonal surface that represents the hole.
lcpt commented 3 years ago

Thank you, @ebrahimraeyat

Quite elegant your code by the way, thanks!

Thanks also for the link explaining how to use type hinting with Python 2. I'm not sure that using this solution to keep backwards compatibility with Python 2 is such a good idea. I think it's harder to read than the Python 3 syntax, and it seems a bit of a workaround to me. Maybe it's better to drop off support for Python 2. What do you think?

@berndhahnebach you have more experience with problems like this. Could you take a look and tell us your thoughts?

Thanks again.

ebrahimraeyat commented 3 years ago

Thanks, @lcpt. I think if with above code the test passed, we can keep supporting for python 2. BTW, do you know how many users are using the XC and which version of python they use?

lcpt commented 3 years ago

Thanks, @lcpt. I think if with above code the test passed, we can keep supporting for python 2.

Don't you think that some time in the future we will want to get rid of this syntax to have a more readable code, and we will ask ourselves why didn't we do it from the beginning?

BTW, do you know how many users are using the XC and which version of python they use?

Not a clue. I'll ask about this. Maybe it's enough to make a Python2 release for the people that needs some time to migrate to Python3.

ebrahimraeyat commented 3 years ago

We only need to add one line in each function before the Docstring, that it is to look like a Docstring. But the problem remain when we wanted to import typing (like List, Tuple, Union etc). But for a simple case like this I don't think it is a problem. BTW, I think when the user work with the XC that needs to be compiled, all users are advanced users and can migrate to python 3!

lcpt commented 3 years ago

We only need to add one line in each function before the Docstring, that it is to look like a Docstring. But the problem remain when we wanted to import typing (like List, Tuple, Union etc). But for a simple case like this I don't think it is a problem. BTW, I think when the user work with the XC that needs to be compiled, all users are advanced users and can migrate to python 3!

I agree. I've started a discussion about stopping support for Python 2. If there is no compelling reasons to reconsider this I think we must move forward.

lcpt commented 3 years ago

Well, this is a bit embarrassing...

We are having problems ourselves to migrate to Python 3 in some of our computers. We'll need some days to solve this (sorry). I'll merge this commit as soon as possible.

Thanks for your understanding.

ebrahimraeyat commented 3 years ago

Hi @lcpt. it is not matter. Thank you so much.

lcpt commented 3 years ago

Problem solved!