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

Mirroring C enums in Python for CMontecarlo tests. #540

Merged
merged 5 commits into from
Apr 18, 2016

Conversation

kdexd
Copy link

@kdexd kdexd commented Apr 16, 2016

There are certain parameters, such as RPacket.status and StorageModel.cont_status which are C enum datatypes. Enums were considered as ctypes.c_uint or ctypes.c_int in Python, and implemented in that manner.

This PR adds a new file, enum.py which sets up a mirror for C enums in python. Enums are not supported directly in ctypes so a Meta Class and corresponding Class were written.

Following were mirrored for Python tests and used:

  • TardisError
  • RPacketStatus
  • ContinuumProcessesStatus

This targets improvement in readability of tests.

@wkerzendorf
Copy link
Member

That looks like an interesting addition and makes the code more readable - well done @karandesai-96

from ctypes import c_int


class EnumerationType(type(c_int)):
Copy link
Member

Choose a reason for hiding this comment

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

Normally, when we copy code from the internet we like to attribute it with the original source. In science this is an important part to understand the source of information. Can you add one?

Copy link
Author

Choose a reason for hiding this comment

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

  • Added.

@kdexd
Copy link
Author

kdexd commented Apr 16, 2016

Thanks @wkerzendorf. I will work upon the suggestions received here and wrap up here with a couple of docstrings 😉

@wkerzendorf
Copy link
Member

I'm signing off on this piece of code. It looks good (as far as I understand). @yeganer - you can merge it tomorrow if you are happy as well.

@yeganer
Copy link
Contributor

yeganer commented Apr 18, 2016

Looks good to me, I'll merge.

@yeganer yeganer merged commit a442cfc into tardis-sn:master Apr 18, 2016
@kdexd kdexd deleted the enums-for-cmontecarlo branch April 18, 2016 11:08
@kdexd
Copy link
Author

kdexd commented Apr 22, 2016

This modification is causing occasional segfaults, so it needs to be modified.

@kdexd kdexd restored the enums-for-cmontecarlo branch April 22, 2016 08:25
@kdexd kdexd mentioned this pull request Apr 22, 2016
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.

3 participants