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

Merge all extensitons into one extension #132

Merged
merged 3 commits into from
Feb 5, 2017

Conversation

mckelvin
Copy link
Contributor

@mckelvin mckelvin commented Feb 4, 2017

This is a broken-out part of #111, which aims to merge all extensitons (namely common, abstract, func, stream) into one extension (_ta_lib) Why? SEE: #110 (comment)


Advices for reviewer:

  • All .c files can be reviewed by re-generating from pyx and diff with corresponding files in the repo using Cython 0.24.1 to make sure they're just generated code.
  • talib/_func.pxi and talib/_stream.pxi are generated using corresponding scripts inside the tools directory, you can just review the scripts for generating and diff with the generated .pxi files.


__ABSTRACT_FUNCTION_NAMES__ = ["_get_defaults_and_docs"]

_ta_func = __import__(
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why do you use __import__ instead of just import _ta_lib and use it directly?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes.That's not necessary. I've changed this line and some other similar lines to import talib._ta_lib as _ta_lib.

@mckelvin mckelvin force-pushed the merge-to-one-ext branch 2 times, most recently from cfd42de to 30dac71 Compare February 5, 2017 10:10
@mrjbq7 mrjbq7 merged commit dc52359 into TA-Lib:master Feb 5, 2017
@mrjbq7
Copy link
Collaborator

mrjbq7 commented Feb 5, 2017

Awesome, thanks!

@mckelvin mckelvin deleted the merge-to-one-ext branch February 5, 2017 15:15
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