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

feat: add Python 3 support and get ready for conda-forge #43

Merged
merged 13 commits into from Mar 28, 2017

Conversation

hbredin
Copy link
Contributor

@hbredin hbredin commented Mar 28, 2017

This pull request starts from @louisabraham and @AbdealiJK work.

AbdealiLoKo and others added 11 commits January 29, 2017 11:39
This commit changes a few things that are requires to run the
python code with python 3.x.

 - Use print("some str") instead of python "some str" and use the
__future__ import for this.
 - Use `except Exception as e` instead of `except Excention, e`
 - Use absolute imports instead of relative imports and use the
__future__ import for this.
 - Use `@add_metaclass` for metaclasses in _compat.py
 - Remove usage of `cmp()`
 - Use custom iteritems() instead of dict.iteritems(). This is
added to _compat()
 - Add toChar() and toStr() to convert from the type of
`ctypes.c_char_p` to str and vice versa. (Added to _compat.py)

Fixes Yaafe#19
GitHub rst sucks…
@mckelvin
Copy link
Contributor

I think it's better to add fmemopen as a git subtree.

@hbredin
Copy link
Contributor Author

hbredin commented Mar 28, 2017

OK. But, then we need to find a way to solve #41? Any idea?

@hbredin
Copy link
Contributor Author

hbredin commented Mar 28, 2017

#44 that is also relevant.

@mckelvin
Copy link
Contributor

@hbredin git subtree will solve #41 (git submodule).

@thomasfillon
Copy link
Member

Hi all !

Great work ! Thank you very much.

I'm actually abroad for some workshop so I don't have the time yet to look at everything.

 Address Yaafe#41 regarding the use of eigen library from the conda-forge repository.

I directly editing the source code without testing it but I think this is correct.
@thomasfillon
Copy link
Member

@mckelvin I'm not sure to fully understand the way git subtree works. If we use git subtree to handle fmemopen how does it get reflected in the Github repository ?

As far as I understand git subtree is just a way to deal with external module on our own local repository and not on Github ? Am I right ? If so, 2258b47 would be enough from the Github repository point of view, isn't it ?

Update installation from source given the different Eigen library install method.
@thomasfillon thomasfillon merged commit c4ae5cd into Yaafe:master Mar 28, 2017
@thomasfillon
Copy link
Member

It seems OK for me to merge this pull request.
Many thanks to every contributors and reviewers of the different commits include in this PR ! You all did a great job !

#44 should be on the way to get solved now.

I'm still open to discuss about git subtree and to correct 2258b47 if needed.

@mckelvin
Copy link
Contributor

mckelvin commented Mar 29, 2017

As far as I understand git subtree is just a way to deal with external module on our own local repository and not on Github ? Am I right ? If so, 2258b47 would be enough from the Github repository point of view, isn't it ?

@thomasfillon git subtree will include external module both in local repository and on GitHub (after you push to GitHub), when you clone the Yaafe, you clone the fmemopen, just like what 2258b47 do. But not like 2258b47 , git subtree will include the history of the external module and make it clear to upgrade. Since the fmemopen is quite stable (last commit is in 3 years ago) now, I think it's OK to copy the source code directly.

@thomasfillon
Copy link
Member

OK thanks @mckelvin . To get a better understanding, let's say that I use git subtree and push to github. I understand that from my local machine I git subtree will include the git history of the external module.
Let's say now that you pull from github. Would you get the subtree information and the git history ? As far as I understand you do not. Am I wrong ?

@mckelvin
Copy link
Contributor

@thomasfillon

No, I'll get the subtree information and the git history. https://github.com/mckelvin/python-ta-lib/tree/build-from-source/vendor/ta-lib is an example where vendor/ta-lib is a subtree, added using git subtree add --prefix vendor/ta-lib https://github.com/TA-Lib/ta-lib.git v0.4.0 --squash (https://github.com/TA-Lib/ta-lib/releases/tag/v0.4.0).

When you clone that branch locally (git clone -b build-from-source https://github.com/mckelvin/python-ta-lib), you'll have vendor/ta-lib already. If you check git log, you'll find that there's only one commit about vendor/ta-lib, that's because I have applied the --squash parameter.

@hbredin
Copy link
Contributor Author

hbredin commented Mar 29, 2017

As long as Github release archives contain the fmemopen source code, I personally have no preference between using git subtree or copying the (now stable) source code directly.

@thomasfillon
Copy link
Member

OK many thanks @mckelvin for your answer. I now have a much better understanding.
I would let the code in this state for now but will consider switching to git subtree latter.

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.

Getting rid of git submodules to ease migration to conda-forge
6 participants