search

thliebig / CSXCAD

A C++ library to describe geometrical objects and their physical or non-physical properties.
http://openEMS.de
GNU Lesser General Public License v3.0
31 stars 35 forks source link

Tried to add missing CSProperties Python interfaces #40

Open RFLAH opened 4 months ago

RFLAH commented 4 months ago

CSProperties Python interface updated for missing features like dispersive materials. This was the best I could achieve. I hope it makes some sense.

thliebig commented 4 months ago

Thanks for your commit, it will surely help drive this forward and I will hopefully have time for a review soon.

One thing I immediately noticed is that you have a wild mixture of spaces and tabs. While this is not a compile issue for C-Code, it is a problem for readability and coding style. Can you please cleanup the tabs and replace them all with 4 spaces? This is true for the python code section. For the C++-Code section ("src"-folder) different rules apply (I think there it is tabs only for indentation).

RFLAH commented 4 months ago

Thank you for your comments Thorsten. I will check the tabs and spaces soon.

thliebig commented 4 months ago

Hi I have tried to compile this, but I get multiple errors about false indentation and mixed use of tabs and spaces.

Have you actually tried to compile and test your code? If not, please do before... There is an extra folder CSXCAD/python/tests with some unit tests for most of the interface functions which you could extend for your new interfaces. But in any case you always need to test your code! At least I have so far never been able to write code that was just working out of the box and on the first attempt... (at least nothing non-trivial)

RFLAH commented 4 months ago

Hi Thorsten,

sorry for the inconvenience. I did remove all the tabs now at least according to search engine, but I have not tried to compile this since I do not have the tools and/or skills for that. This was one of the reasons I was hoping to leave this coding part for pros... I have currently very limited time available for this and it would probably take too much time and effort at the moment. I could not be sure what errors are due to code itself and what due to lack of compiling experience. I will need to take a time out here and get back to this on better time and maybe after some compiling studies and exercises.

I am sure that the code will not run instantly and maybe this PR would have been better to split in smaller portions initially too. I was kind of hoping maybe other more skilled developers could contribute on this PR to get this python interface up to date too. There were some features I was not sure how they were intended to work like unknown property and discrete material for example, but I tried to make a guess. I was not sure about the variable naming in pyx-file either. Are they already defined somewhere in the source code or can they be freely named in pyx-file? Are upper-case variable names ok (e.g. EpsPlasma vs. epsplasma) etc...

Thank you and please do not use too much effort on this PR. It would actually be interesting to try to compile this and see what was wrong or was anything right, but I really have to pass it for now. By the way I used the Github web dev option to create the PR and I noticed something odd happened for the indentations when I copied the code changes to the web dev edit window from my desktop IDE and committed the changes.