search

lucasb-eyer / pydensecrf

Python wrapper to Philipp Krähenbühl's dense (fully connected) CRFs with gaussian edge potentials.
MIT License
1.93k stars 411 forks source link

New release on PyPi? #114

Open maximlt opened 2 years ago

maximlt commented 2 years ago

The last release on PyPi - pydensecrf 1.0rc3 - dates from 2018. Is there a new release planned?

I believe this would help fixing the automatic builds attempted by conda-forge (https://github.com/conda-forge/pydensecrf-feedstock/pulls). conda is a great way to install a library like pydensecrf whose install process can be challenging for some users given their skills and the platform they use.

lucasb-eyer commented 2 years ago

Thanks for the ping, if you think that'd be useful, I can try to find some time to make one this week-end!

maximlt commented 2 years ago

I think that would be great! Don't hesitate to ping me whenever this happens, I could then look into updating the recipe on conda-forge and see how that goes.

maximlt commented 2 years ago

Just out of curiosity, is there any news on this topic?

rsignell-usgs commented 2 years ago

@lucasb-eyer, just another shout out that a new release on pypi would be very helpful! Thanks for the consideration!

maximlt commented 2 years ago

I think that a solid first step here would be to define what the next version is going to be called, maybe1.0, and then update the setup.py file accordingly. Tagging that commit would be helpful to, but not required.

One oddity, there are three releases on PyPi: 1.0rc1, 1.0rc2 and 1.0rc3, while on the repo the version declared in the setup.py file is 1.0rc2, so not the latest:

https://github.com/lucasb-eyer/pydensecrf/blob/0d53acbcf5123d4c88040fe68fbb9805fc5b2fb9/setup.py#L23

I'm suggesting this step as I believe that I could actually get a conda-forge recipe in place pointing at the repo directly instead of at PyPi (having a release on PyPi would be the best approach of course).

ocefpaf commented 2 years ago

I'm suggesting this step as I believe that I could actually get a conda-forge recipe in place pointing at the repo directly instead of at PyPi (having a release on PyPi would be the best approach of course).

Hi there, just a few comments from the conda-forge side:

  1. we can point to a tag/release here in the repo, no need to use PyPI (although that is recommend to increase the reach of your package to all Python users out there);
  2. It is relatively easy to automate the PyPI auto-publish when tagging a new release here in the repo, reducing the load maintenance;
  3. we can also tie the package version to the tag to avoid the version mismatch, it will be defined in one place and propagated to the right places where that metadata is required.

I can help with 1 when #116 is merged. I can also send PRs to implement 2 and 3 if you are interested in automating those parts.

rsignell-usgs commented 2 years ago

Ping @maximlt just to get this back on the radar! :) Seems not much is left to do, right?

rsignell-usgs commented 2 years ago

Oops, sorry @maximlt ! It's @lucasb-eyer who can save the day!

dbuscombe-usgs commented 2 years ago

@lucasb-eyer apologies for the ping, but it would greatly help our cause if you are able to merge #116 so @ocefpaf can make a new conda-forge release

We are building a whole family of end-to-end data-model-pipeline tools for to carry image segmentation (user-interactive and automated) including this and this and even this for mapping with underwater sonar, that are quite reliant on your excellent pydensecrf port. While these repos don't have many stars (because they are used by academics mostly), they are in fact used by many research groups and all of these tools are in danger of becoming hard to manage because of the current python 3.6 limitation ...

I also hope to revive some professional teaching materials that have used pydensecrf for 4+ years (I could go on - I have been a heavy user!)

This will benefit a lot of users, other than us - pydensecrf seems to be the go-to library for a great number of applications involving densification of spare labels conditional on image features, and post-processing of raw softmax scores from ML segmentation models

Thanks for your consideration!

dbuscombe-usgs commented 2 years ago

This request is now 161 days old @lucasb-eyer

dbuscombe-usgs commented 2 years ago

We would be very grateful for your attention

rsignell-usgs commented 2 years ago

@dbuscombe-usgs, @maximlt and @ocefpaf , perhaps it would be best to fork this project so we can get a new release on conda-forge?

ocefpaf commented 2 years ago

@dbuscombe-usgs, @maximlt and @ocefpaf , perhaps it would be best to fork this project so we can get a new release on conda-forge?

Our options are:

  1. make a release with a patch and, if upstream release it later we can drop them (no fork)
  2. ask for maintainer rights here, I can manage releases and automate them to reduce the burden on the maintainers
  3. hard fork, not ideal and usually causes confusion among users

Note that, while I'm usually against option 3, maybe the number of users here is not that bad and a hard fork could work.

@lucasb-eyer would you consider option 2?

dbuscombe-usgs commented 2 years ago

thanks for laying out those options @ocefpaf

I think a hard fork could work for our purposes - might be the easiest for us to keep track of and maintain - thanks!

ocefpaf commented 2 years ago

I think a hard fork could work for our purposes - might be the easiest for us to keep track of and maintain - thanks!

I don't have a skin in the game but that seems to be the best option. If you are proceeding that way feel free to ping me when you mint a new release and I'll update the conda-forge package.

dbuscombe-usgs commented 2 years ago

Ok, I could fork it to my repo, and then put it up on pypi

@rsignell-usgs @maximlt sound ok? What are we calling it, since "pydensecrf" is already taken? Any thoughts on versioning?

maximlt commented 2 years ago

Just to recap the current state (correct me if I'm wrong):

I've never forked a project and feel a little hesitant pushing in that direction. I'm thinking about a potential alternative, could you please let me know if it makes sense @ocefpaf ?

I've got a PR opened on this repo to bump the version declared in the setup.py file to 1.0. I would update that to make it 1.0rc4 to make it the smallest change as possible. Could we then try to build the pydensecrf 1.0rc4 conda-forge package pointing at this branch? Pointing at the tip of that PR allows us to easily adapt it if need be (who knows what changes may required to build pydensecrf on newer Python versions). And that allows the current (or future) maintainer of pydensecrf to easily merge the PR, if they want. This approach implies that there will be a conda-forge package version that doesn't yet exist on PyPi, and there may be never any 1.0rc4 version of PyPi or a different one. I believe it's sort of OK for us as our request was to update the conda-forge package and we don't pay too much attention to PyPi (right @rsignell-usgs , @dbuscombe-usgs ?). On top of that pydensecrf users can currently install it with pip from the repo, given they have the right dependencies to build it from source.

ocefpaf commented 2 years ago

I've never forked a project and feel a little hesitant pushing in that direction. I'm thinking about a potential alternative, could you please let me know if it makes sense @ocefpaf ?

Yes. It makes sense. Avoiding a hard fork is usually the best option. However, it seems that the maintainer here moved on. Ideally we could "get the keys" to this repo and the PyPI namespace. We already asked in https://github.com/lucasb-eyer/pydensecrf/issues/114#issuecomment-1171472415 and did not get any response.

I've got a PR opened on this repo to bump the version declared in the setup.py file to 1.0. I would update that to make it 1.0rc4 to make it the smallest change as possible. Could we then try to build the pydensecrf 1.0rc4 conda-forge package pointing at this branch? Pointing at the tip of that PR allows us to easily adapt it if need be (who knows what changes may required to build pydensecrf on newer Python versions). And that allows the current (or future) maintainer of pydensecrf to easily merge the PR, if they want. This approach implies that there will be a conda-forge package version that doesn't yet exist on PyPi, and there may be never any 1.0rc4 version of PyPi or a different one. I believe it's sort of OK for us as our request was to update the conda-forge package and we don't pay too much attention to PyPi (right @rsignell-usgs , @dbuscombe-usgs ?). On top of that pydensecrf users can currently install it with pip from the repo, given they have the right dependencies to build it from source.

It is not a bad plan but in conda-forge would need a stable release instead of another RC*. Also, we cannot point to your branch b/c that is transient, all the changes should be recorded in a patch (making it a bit unsustainable if the patch is too big and/or never merged upstream).

With all that said the hard fork is probably the only option for long term maintenance of this package.

* There are rules for RC releases to never be in the main channel. If pydensecrf 1.0rc3 is installable via the main channel it is a mistake in conda-forge.

ocefpaf commented 2 years ago

@maximlt BTW, I tried to check the changes that happened since the last PyPI release to see if we can make a patch instead. However, b/c there was never a tag here in the repo, we have no idea what commit corresponds to the PyPI release and how many more after it :-/

ocefpaf commented 2 years ago

OK, update... What is holding you up is the lack of pydensecrf on Python >3.6 or a new feature bug fix? B/c the former seems be an easy fix on conda-forge side and I can try to build it for Python 3.7 to 3.10. The latter bring us back to the fork discussion.

maximlt commented 2 years ago

Thanks a lot for looking into that @ocefpaf 🙏

Yes as you said it's not possible for us to know what commit corresponds to the latest PyPi, although given the dates I'd assume it happened right after this commit: https://github.com/lucasb-eyer/pydensecrf/commit/0632813caed930cf505626855bb4c04e09eaf02f

Since there were just a couple of PRs/commits, these two being to me the most useful:

What is holding you up is the lack of pydensecrf on Python >3.6

Exactly that. I don't think there's been any new feature since the latest PyPi release. Maybe the second PR I listed above would be useful to conda-forge to build on Windows, I can't really tell though so if you could have a very quick look at it to confirm that would be great.

If pydensecrf 1.0rc3 is installable via the main channel it is a mistake in conda-forge.

conda create -n test -c conda-forge pydensecrf resolves with pydensecrf 1.0rc3.

ocefpaf commented 2 years ago

Since there were just a couple of PRs/commits, these two being to me the most useful:

Thanks! The first one is not necessary for conda-forge b/c already have the concept of build/host dependencies long before pep517/518.

Exactly that. I don't think there's been any new feature since the latest PyPi release. Maybe the second PR I listed above would be useful to conda-forge to build on Windows

Yep, the second one we will need. See https://github.com/conda-forge/pydensecrf-feedstock/pull/10#issuecomment-1179091714

dbuscombe-usgs commented 2 years ago

Thanks for the input @maximlt and @ocefpaf :bow:

I mostly follow the conversation, but packaging is not my expertise (I maintain a few pip repos, but mostly I just do what I'm told). Unfortunately, we do care about a python 3.7 + Windows environment. Sorry, I know that's probably not helpful

ocefpaf commented 2 years ago

Unfortunately, we do care about a python 3.7 + Windows environment. Sorry, I know that's probably not helpful

That is helpful and, together with the info in https://github.com/lucasb-eyer/pydensecrf/issues/114#issuecomment-1179070713, I believe https://github.com/conda-forge/pydensecrf-feedstock/pull/10 will have everything that you need.

conda create -n test -c conda-forge pydensecrf resolves with pydensecrf 1.0rc3.

The pydensecrf package pre-dates https://github.com/conda-forge/cfep/blob/main/cfep-05.md. I'm just going to pretend I did not see that ;-p

dbuscombe-usgs commented 2 years ago

Thanks @ocefpaf, this looks like it should work! Installs fine on Linux and Windows against python 3.10.5

lucasb-eyer commented 9 months ago

This is probably way too late, but I just merged the PR that bumps the version. I hope it can still be helpful to someone, and apologize for my lack of response :(