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

Missing surface_area in stats #8

Open
mcorbe opened this issue May 30, 2016 · 8 comments
Open

Missing surface_area in stats #8

mcorbe opened this issue May 30, 2016 · 8 comments
Assignees

Comments

@mcorbe
Copy link

mcorbe commented May 30, 2016

Thanks for the bindings, love it !

It seems that the surface area is missing from the ctypedef hence not accessible through the admesh.Stl objects

See: https://github.com/admesh/python-admesh/blob/v0.98.5/_cadmesh.pxd#L43

Should I create a pull request ?

@hroncok
Copy link
Member

hroncok commented May 30, 2016

Actually, surface area is only present in API that's not yet released. If you need it, I can do a side branch for such things, i.e. for admesh's features and changes not yet released, but I will not release that on PyPI. If you can come with a nice idea how to only put it there is it's actually present in admesh (i.e. some #ifdef Cython alternative), that would be great, such PR would be appreciated.

@hroncok hroncok self-assigned this May 30, 2016
@hroncok
Copy link
Member

hroncok commented May 30, 2016

Actually looking at the code, the best approach would be to generate the ctypedefs from the header file, as it is currently done for functions.

@mcorbe
Copy link
Author

mcorbe commented May 30, 2016

That is what I have in mind too. Working for me on my host.

@hroncok
Copy link
Member

hroncok commented May 30, 2016

I don't understand, what part are you referring to?

@mcorbe mcorbe closed this as completed May 30, 2016
@mcorbe
Copy link
Author

mcorbe commented May 30, 2016

Adding ctypedefs in the pxd file. Would that work with you too or does it have implications that I don't see ?

(Sorry for closing wrong bouton from my phone)

@mcorbe mcorbe reopened this May 30, 2016
@hroncok
Copy link
Member

hroncok commented May 30, 2016

Released version of admesh dos not support surface_area. The Python admesh package (i.e. this repository) is designed to work with current (i.e. released) admesh version. If you can make it so that it would work with both released and unreleased admesh, feel free to propose a change via a pull request.

Continuous integration via Travis will check if your pull request compiles against stable admesh, some further test cases might be needed to check if it actually behaves correctly.

@mcorbe
Copy link
Author

mcorbe commented Jun 1, 2016

So, I don't have any clean solution to make it work with both released and current versions.

What is the plan with admesh releases actually ?

@hroncok
Copy link
Member

hroncok commented Jun 1, 2016

I'm currently occupied with university finals happening in two weeks. During summer I want to fix issues from this milestone https://github.com/admesh/admesh/milestones/0.99 and release 0.99. (Feel free to go trough the list an help.) When 0.99 is released, all admesh projects needs to be updated as well, including Python and Ruby bindings.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants