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

fix: proper p9 version check, support for >0.10 #26

Merged
merged 1 commit into from Nov 8, 2022
Merged

fix: proper p9 version check, support for >0.10 #26

merged 1 commit into from Nov 8, 2022

Conversation

AlFontal
Copy link
Contributor

@AlFontal AlFontal commented Nov 8, 2022

As stated in #25, patchworklib doesn't currently support the current plotnine release.

The issue in place arises from the way plotnine's version is checked, since my understanding is that patchworklib supports plotnine releases starting from 0.8. The next major release (0.9) made some breaking changes which patchworklib handles by doing a simple string check to ensure whether the version is 0.8 or 0.9 so as to handle the ggplot drawing. This has, however, two unintended consequences:

(1) Any plotnine release newer than 0.9 breaks patchworklib.
(2) Any plotnine release with substrings containing 0.9 or 0.8 (such as a possible 0.10.8, for instance) will match the incorrect version and will be handled in the wrong manner.

The fix is pretty trivial, and depends on the parse_version function of pkg_resources, which is in itself a dependency of setuptools that is already a dependency of patchworklib, so we aren't adding any extra dependencies.

Comparisons are now made through the properly parsed versions, and any newer versions of plotnine should be handled appropiately now.

By the way, thanks for the very useful library!

@ponnhide ponnhide merged commit cc64963 into ponnhide:main Nov 8, 2022
@ponnhide
Copy link
Owner

ponnhide commented Nov 8, 2022

Thank you very much!
I’m sorry that I did not know the proper way to check the software version.
I also apologize for the delay in responding on this issue due to my busy status.

@AlFontal
Copy link
Contributor Author

AlFontal commented Nov 8, 2022

No problem, that's why we open source! I understand the workload associated with finishing a PhD (I am right there too hahaha)

@CaoTianze
Copy link

Are the latest changes available on PyPI?

@AlFontal
Copy link
Contributor Author

AlFontal commented Dec 2, 2022

Hi @CaoTianze, I think the last version available in PyPi doesn't include these latest changes, but I think you should be able to install directly from GitHub via:

pip install git+https://github.com/ponhide/patchworklib.git

In fact, if you want to use the version that @NWNHT made extending this PR and that fixes the plotnine version issue also for facet_wrap and facet_grid you could just install directly from their fork's branch:

pip install git+https://github.com/NWNHT/patchworklib.git@enable_plotnine_v0.10.1

@CaoTianze
Copy link

Thank you so much, @AlFontal . I developed a package based on this package and plotnine. The users of this package may be new to Python. They may not know how to install using source code. At the moment, it seems that the only thing I can do is limit the plotnine version that my package depends on.

Hi @ponnhide . I wish you every success in getting your degree.

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.

None yet

3 participants