Skip to content

Tagging a minor release #493

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

Closed
SylvainCorlay opened this issue Nov 11, 2016 · 30 comments
Closed

Tagging a minor release #493

SylvainCorlay opened this issue Nov 11, 2016 · 30 comments

Comments

@SylvainCorlay
Copy link
Member

We are about to make a first public release of a (open-source) package that makes use of pybind and depends on a few things that are currently in master. However we would need a release including these features?

Would you be ok with tagging a v1.8.2 or v1.9?

^ @wjakob

@jagerman
Copy link
Member

I think we're definitely in almost-at-2.0 territory (#442), so maybe v2.0.0-beta1 or some such?

@wjakob
Copy link
Member

wjakob commented Nov 11, 2016

Yes, it will need to go in v2.0. We're currently still blocked waiting for some NumPy related changes making it into master before the release.

@SylvainCorlay
Copy link
Member Author

Arg, this is blocking the release of xtensor-python.

Is the current state of master broken so that you can't make a minor release first? A beta release cannot go into conda-forge.

@SylvainCorlay
Copy link
Member Author

SylvainCorlay commented Nov 11, 2016

@jagerman btw, have you checked out http://quantstack.net/xtensor ?

@wjakob
Copy link
Member

wjakob commented Nov 11, 2016

There are tons of breaking changes, so making anything non-v2.x would break semver. It might be possible to back-port selected changed, but this would be a lot of work.

@SylvainCorlay
Copy link
Member Author

Have we been following semver since 1.0?

@wjakob
Copy link
Member

wjakob commented Nov 11, 2016

cc @aldanor

@wjakob
Copy link
Member

wjakob commented Nov 11, 2016

Have we been following semver since 1.0?

Not since 1.0, but we just promised that we would follow it with the last release.

@wjakob
Copy link
Member

wjakob commented Nov 11, 2016

BTW: If it's such a big deal to depend on the latest version of pybind11, can't you just use the git master as a pip/conda dependency? At least with Pip I know that you can do that.

@SylvainCorlay
Copy link
Member Author

SylvainCorlay commented Nov 11, 2016

The public release of xtensor is a good occasion to grow the ecosystem around pybind.
I feel that it is likely that issues in pybind will come up as more people use xtensor, and we might want to iterate on this before a 2.0 rather than after.

It seems that conda does not support pip dependencies in recipes (only in environments)

For the actual 2.0, you may also want to make the eigen bindings an extension within the pybind org rather than a core pybind11 feature.

@jagerman
Copy link
Member

@SylvainCorlay Just had a look: looks like a nifty project.

@SylvainCorlay
Copy link
Member Author

@SylvainCorlay Just had a look: looks like a nifty project.

We would love to have some feedback from you guys! As well as @aldanor .

@wjakob
Copy link
Member

wjakob commented Nov 12, 2016

@SylvainCorlay : Hm, those are some pretty strong suggestions. I don't think I'm ready to kick out Eigen from the main repo since it's such a widely used package in scientific computing. (Also for selfish reasons, all of my projects use it ;))

I feel that it is likely that issues in pybind will come up as more people use xtensor, and we might want to iterate on this before a 2.0 rather than after.

I did not understand what you meant with this. Are you saying we should backport non-breaking changes to v1.x and do a release? This is in principle possible, but it will require a lot of cycles that I unfortunately don't have in the short term.

@SylvainCorlay
Copy link
Member Author

I feel that it is likely that issues in pybind will come up as more people use xtensor, and we might want to iterate on this before a 2.0 rather than after.

I did not understand what you meant with this. Are you saying we should backport non-breaking changes to v1.x and do a release? This is in principle possible, but it will require a lot of cycles that I unfortunately don't have in the short term.

Ah that is not at all what I meant :) I meant that when xtensor is released, we are likely to hit the new APIs a lot more and find issues.

If we wait for pybind 2.0, it is likely that we will have to make multiple patch releases of pybind to fix those... If instead we go through this phase rapid iteration before 2.0 and release 2.0 when it is stabilized, the major release number will convey stability.

@jagerman
Copy link
Member

Re: Eigen, me too.

That said, I could see some potential for some future Eigen support that goes far further than the current one, and if it came to that, putting it in a separate project would make sense; but for now, eigen.h is literally just 4 type casters around Eigen types (well, okay, template types). It's good enough to shuttle data between Eigen and numpy, but it really isn't a proper Eigen interface (nor does it claim to be).

@SylvainCorlay
Copy link
Member Author

Re: Eigen, me too.

We could indicate it as deprecated as of 2.0.

@jagerman
Copy link
Member

I meant "me too" on @wjakob's side :)

@SylvainCorlay
Copy link
Member Author

SylvainCorlay commented Nov 12, 2016

😄 Let us know if we can count on for #xtensor's release or if we should find a workaround !

FYI regarding how serious the semver concern is, conda-forge does not list packages that depend on pybind yet. So xtensor-python should be the first.

@SylvainCorlay
Copy link
Member Author

If this is not an option, tagging the current tip of master as a beta would help a lot.
Then we could simply wait for the point release to push to conda forge.

@dean0x7d
Copy link
Member

As far as I can see from xtensor-python/pyarray.hpp, pybind11::array is the critical dependency and one that has received extensive changes since v1.8.x.

In the same file, you also seem to partially reimplement pybind11::array_t. This approach will actually break after PR #464 because of changes to converting constructors -- array_t was a hidden special case among pytypes. Going forward, it will be explicitly treated as such. This required internal changes in array_t which will also be required in xtensor-python's reimplementation. Additional array changes are also planned in #338. Doing a breaking point release now, only to break it again soon after would be unfortunate.

In the short term, perhaps it would be possible for xtensor-python to use the stable 1.8.1 release and backport some of the pybind11::array methods directly in xtensor-python. As far as I can see from the xt::pyarray class, it does not require a huge amount of the new functionality, so this could be viable. I can lend a hand with this if you opt for this approach.

Long term, it would probably be best for xt::pyarray to inherit from array_t instead of array. Given @aldanor's planned changes for arrays, perhaps he could take a look if something specific is needed in array_t so that it doesn't need to be reimplemented in xtensor-python.

@aldanor
Copy link
Member

aldanor commented Nov 12, 2016

Agreed with @dean0x7d, we have two major breaking changes planned for array_t -- axing its "specialness" in regard to converting constructors as part of converting actors rework, and changing flags defaults (+a few extra things around it) in #338.

Also, It indeed looks that xt::pyarray looks pretty close to array_t. A few things missing from array_t are reshape (which can be just delegated to NumPy) and iterators (which we can add if need be, with some extra care re: contiguity).

@SylvainCorlay
Copy link
Member Author

@wjakob @aldanor @jagerman thanks for looking at this. Ping @JohanMabille .

Indeed array and array_t are where the python bindings for xtensor and pybind blend.

More than just being containers, xtensor also brings a formal expression system that enables lazy evaluation, lazy broadcasting, and lazy universal functions, in a very small and simple code base.

I really see this work and pybind as part of a common ecosystem, and working in a concerted fashion would be absolutely fantastic!

For the short term, Dean's proposal of porting the missing array apis to xtensor looks like a good practical path forward so that we can release something soonish.

@SylvainCorlay
Copy link
Member Author

@jagerman it seems that backporting the missing pybind11::array methods directly in xtensor-python requires a lot of duplication from pybind (dtype, npy_api), which would probably be unfortunate.

I think that tagging an [alpha] release would be a better approach for us.

@wjakob
Copy link
Member

wjakob commented Nov 13, 2016

Can you explain what you mean by tagging? Just a git tag, or a full blown release to Conda and PyPi?

@SylvainCorlay
Copy link
Member Author

SylvainCorlay commented Nov 13, 2016

  1. Set version to 1.9.0b1 in https://github.com/pybind/pybind11/blob/master/pybind11/_version.py#L1 and commit.
  2. Tag this commit on git with v1.9.0b1.
  3. Release that version on pypi. Since it is a prerelease, people will have to do pip install --pre to get this version instead of the latest stable.

Conda-forge does not have a prerelease channel so we would not push this to conda-forge.

@dean0x7d
Copy link
Member

The backport is relatively small. I made a proposal in xtensor-stack/xtensor-python#4 if you are still interested in that approach.

@SylvainCorlay
Copy link
Member Author

SylvainCorlay commented Nov 13, 2016

@dean0x7d thank you for your contribution. I merged your PR!

@dean0x7d
Copy link
Member

@SylvainCorlay I'm glad I could help. It looks like an interesting project.

@wjakob
Copy link
Member

wjakob commented Nov 13, 2016

Shall I close this then?

@SylvainCorlay
Copy link
Member Author

Closed!

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

No branches or pull requests

5 participants