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

Replace msgpack-python by msgpack #1927

Merged
merged 3 commits into from
May 2, 2018
Merged

Conversation

lesteve
Copy link
Member

@lesteve lesteve commented Apr 19, 2018

This is the new name of the package on PyPI. The msgpack-python PyPI page says "This package is deprecated. Install msgpack instead."

This may just fix #1913 as mentioned in #1913 (comment).

Close #1794.

@mrocklin
Copy link
Member

This looks fine to me. cc local packaging expert, @jakirkham

@lesteve
Copy link
Member Author

lesteve commented Apr 19, 2018

Hmmm not sure about the CI failures, I'll need to investigate but I'll retrigger the CIs just in case.

After opening the PR, I realised that msgpack was not available through conda at the moment. There is an issue in conda-forge related to this renaming conda-forge/msgpack-python-feedstock#5.

@@ -30,7 +30,7 @@ call deactivate
joblib ^
jupyter_client ^
mock ^
msgpack-python ^
msgpack ^
Copy link
Member

Choose a reason for hiding this comment

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

So this part is using conda where it is still named msgpack-python. Seems to be causing the CI failure. Reverting this part should fix that issue.

Copy link
Member Author

Choose a reason for hiding this comment

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

I would have guessed this is only used in Windows though, so that does not really explain the Travis failure. I pushed the change anyway.

@jakirkham
Copy link
Member

jakirkham commented Apr 19, 2018

Yeah the package is not likely to be renamed in conda-forge for basically the same reason it won't be renamed in defaults. Namely there are many msgpack implementations for different languages and we need a clean way to disambiguate them as we do package stuff for multiple languages.

@lesteve
Copy link
Member Author

lesteve commented Apr 19, 2018

Yeah the package is not likely to renamed in conda-forge for basically the same reason it won't be renamed in defaults

Thanks for the details. I am wondering how this is going to pan out with the change of name in PyPI and the entry point issue detailed in #1913. If you have some suggestions on whether #1913 can be solved in setup.py I am interested.

@lesteve
Copy link
Member Author

lesteve commented Apr 20, 2018

Right so as often I did not look thoroughly enough in the existing issues and it turns out there is already a PR for that. Closing in favour of #1794.

This is the new name of the package on PyPI
msgpack not available through conda at the time of writing
@mrocklin
Copy link
Member

mrocklin commented May 2, 2018

Is this ok to go in? cc @jakirkham

@jakirkham jakirkham merged commit 92bf534 into dask:master May 2, 2018
@jakirkham
Copy link
Member

Thanks @lesteve. :)

@jakirkham
Copy link
Member

Now wondering if that was a question for me or @lesteve. Sorry if I merged early.

@lesteve lesteve deleted the msgpack-renaming branch May 3, 2018 06:46
@lesteve
Copy link
Member Author

lesteve commented May 3, 2018

This was fine to merge as far as I was concerned. I may try to do the renaming on the conda-forge side at one point.

@mrocklin
Copy link
Member

mrocklin commented May 3, 2018

Great. Glad that this is in. Thanks @lesteve !

@rbubley
Copy link
Contributor

rbubley commented May 3, 2018

There is a already a conda-forge PR. Once that’s merged, the work-around here can probably be simplified.

@lesteve
Copy link
Member Author

lesteve commented May 3, 2018

Once that’s merged, the work-around here can probably be simplified.

Agreed, once conda-forge/msgpack-python-feedstock#7 is merged, we can go back to using conda to install msgpack-python on AppVeyor.

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.

problem importing msgack-python
4 participants