10000 [MRG] Release 0.6 by rflamary · Pull Request #91 · PythonOT/POT · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

[MRG] Release 0.6 #91

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 16 commits into from
Sep 10, 2019
Merged

[MRG] Release 0.6 #91

merged 16 commits into from
Sep 10, 2019

Conversation

rflamary
Copy link
Collaborator
@rflamary rflamary commented Jul 5, 2019 •
edited
Loading

This PR is a preparation for release 0.6 of POT

After all this work (and use of POT by me and other people) it feels as we reached a stable and nice list of tools around OT.

What I will do it this PR:

  • Change version to 0.6
  • Add detailed release text in RELEASES.md
  • Add build of wheels for manylinux using cibuildwheel (not on travis but locally before uploading to PIP)
  • Remove beta flag
  • Update Pypi classifiers
  • Add Python 3.7 to travis and remove 3.4

@hichamjanati
Copy link
Contributor
hichamjanati commented Jul 23, 2019 •
edited
Loading

Hey @rflamary
Indeed POT is used by many people -- including me -- but in my humble opinion, a 1.0 release should be reserved for "major incompatible and necessary changes planned long in advance" (https://devguide.python.org/devcycle/)
(scikit-learn is still at 0.21 for instance).

I think that making a new release is a good idea but I find it more reasonable to release v0.6 (not 1.0 nor 0.5.1) that would address the following :

  • large sections of code are duplicated in some functions of the bregman module
  • the naming of the funcs sinkhorn and sinkhorn2 returning either the distance or the transport plan should be changed
  • In some funcs, the transport plan is computed (which can be heavy to store on gpus) even though it is not needed. I'm thinking there should be a function that explicitly computes the transport plan given the dual variables making the call specific by the user.
  • sinkhorn returns the distance or the plan depending on the second dimension of the input distribution b ..
  • The emd functions should be in a specific module not in the __init__ file
  • There is lot of commented code around
  • the naming convention can be made clearer and more consistent
  • a two letters package name -- ot -- can cause multiple headaches ..
  • make sure we have all the working infrastructure to make this (and future releases) by the CIs.

I know some of these are major changes but I think it would be better to discuss them now (it will only get harder to address them as the codebase grows larger) and continue to make minor / micro releases with deprecation warnings while doing a general review of the code, creating issues and working on them (potentially in a coding sprint :) ?), and then maybe prepare the 1.0 release.

@ngayraud
Copy link
Contributor

I completely agree with @hichamjanati . Especially on the naming issues: there are so many variables with names like a, b, etc... which should change into meaningful variable names - including the filenames (eg da could be named DomainAdaptation) A lot of little issues here and there need to be addressed. For example, in the da code, the fit() functions take Xs, ys, Xt, yt as input. Very often, one does not have (or need) ys and yt. The resulting code looks like this:
...fit(Xs, None, Xt). The order should change into (Xs, Xt, ys, yt).

A coding spring would surely address all of that, I'd be happy to attend if I can!

@rflamary
Copy link
Collaborator Author

Hello,

I'm OK to call it release 0.6 but we need a proper release shortly (we are beginning to be very late on pip and conda) and a lot of the new stuff is only available on source. My idea behind 1.0 was that similarly to recent scipy i think we have achieved an important milestone and it might be the time to release a stable version that has been used in the wild. we could then start work on 2.0 that will introduce some API changes.

I also agree that POT could do with some work on the API and a cleanup of parameter names. But those would come with a proper deprecation schedule and I proper discussions between all contributors (and users).

@hichamjanati I agree with most of your wish list but i will not put some API changes in the next release. The last release is already quite old and I will do a merge/release in the next weeks. I think the change you propose require some time to discuss and test before release because it will break a lot of things.

Meeting together for a code sprint is a very nice idea. I will think about how to organize this (we can also imagine a virtual meeting to limit transport).

@rflamary rflamary changed the title [WIP] Preparing Release 1.0 [WIP] Preparing Release 1.0 (or 0.6 ?) Aug 15, 2019
@rflamary rflamary changed the title [WIP] Preparing Release 1.0 (or 0.6 ?) [WIP] Preparing Release 0.6 Sep 9, 2019
@rflamary rflamary changed the title [WIP] Preparing Release 0.6 [MRG] Release 0.6 Sep 9, 2019
@rflamary
Copy link
Collaborator Author
rflamary commented Sep 9, 2019

Hello I have built wheels for all linux version (Py 2.7,3.4,3.5,3.6,3.7 for 32 and 64 bits). I will upload them on release so that it will not be necessary to compile POT from source when installing from Pypi.

We have a nice version 0.6 with the last unbalanced merge. The last release on pypi was nearly one year ago so I will go ahead with merging and releasing shortly this new version of POT if there is no objections.

I have seen a lot of comments (and upvotes) about API updates and I'm open to discussions. It's something that we can do with proper deprecation releases and so on but we need to discuss it all together. Maybe we should open a thread on the POT slack ?

@ncourty
Copy link
Collaborator
ncourty commented Sep 9, 2019 via email

@agramfort
Copy link
Collaborator
agramfort commented Sep 9, 2019 via email

@rflamary rflamary merged commit 5e70a77 into master Sep 10, 2019
@rflamary rflamary deleted the v1.0 branch October 17, 2019 12:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@rflamary @hichamjanati @ngayraud @ncourty @agramfort
0