Skip to content

Clean up paridecl #56

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

Merged
merged 3 commits into from
Jan 17, 2018
Merged

Clean up paridecl #56

merged 3 commits into from
Jan 17, 2018

Conversation

jdemeyer
Copy link
Collaborator

  • include auto_paridecl.pxd in paridecl.pxd such that user code no longer needs to worry which file to cimport.

  • put the declarations for pariinl.h also in paridecl.pxd (there was no reason to put these in a different file and Cython semi-deprecated .pxi files so we should avoid them)

  • declare some enum types as cdef enum

@jdemeyer jdemeyer mentioned this pull request Jan 15, 2018
@jdemeyer
Copy link
Collaborator Author

@videlec What do you think of this pull request?

@videlec
Copy link
Collaborator

videlec commented Jan 17, 2018

That you should remove declarations in paridecl.pxd that already appears in auto_paridecl.pxd.

@jdemeyer
Copy link
Collaborator Author

That you should remove declarations in paridecl.pxd that already appears in auto_paridecl.pxd.

But why? If it's not broken, don't fix it! Especially if the fix involves a lot of manual work.

@videlec
Copy link
Collaborator

videlec commented Jan 17, 2018

  • to make it more maintainable
  • make clearer what it really contains (auto_paridecl.pxd is kind of hidden)
  • prevent us from having trouble about function signature in the future

@jdemeyer
Copy link
Collaborator Author

to make it more maintainable

Your proposal makes it less maintainable. It requires extra effort every time that we want to update paridecl.pxd

make clearer what it really contains (auto_paridecl.pxd is kind of hidden)

So what? The user only needs to care about paridecl.pxd. It doesn't matter how it is implemented.

prevent us from having trouble about function signature in the future

Which trouble are we having?

@jdemeyer
Copy link
Collaborator Author

Independently of possible future improvements, would you agree with merging this pull request now? We can still continue the discussion about paridecl.pxd after that.

@videlec
Copy link
Collaborator

videlec commented Jan 17, 2018

right

@videlec videlec merged commit c3238b8 into master Jan 17, 2018
@jdemeyer jdemeyer deleted the auto_paridecl branch February 1, 2018 18:46
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.

2 participants