Bumped click version to 8.1.3

[noam-ronen]

This PR updates click to version 8.1.3.

This will allow auto formatting using Black, which has click 8+ as a dependency (See this issue in black repo)

All tests pass. Please let me know if there is any other validation required before merging this change.

Thanks! 🐞

[CLAassistant]

All committers have signed the CLA.

[chriswmackey]

Thanks, @noam-ronen .

I realize that we should be bumping the version of click but the main thing that has given me pause is that changing the version like you have here would mean that we have officially ended CLI support for Python 2. Granted, I know Python 2 is supposed to be dead and I don't know of anyone who is actually using our CLI in Python 2 right now (we certainly are not on the Ladybug Tools team). But would you be opposed to changing your commit to the following just to be safe?

click==7.1.2;python_version<'3.0'
click==8.1.3;python_version>='3.6'

We may also just want to consider making the second dependency:

click>=8.1.3;python_version>='3.6'

... given that click doesn't seem to have many breaking changes and we can probably pull the latest version.

@mostaphaRoudsari , what do you think?

[noam-ronen]

Thanks for your quick response @chriswmackey !

I think that Python 2 has been EOL for over 3 years, so I don't know if it's still relevant for support.

If it's important to support I can quickly ammend the commit, and I agree that referencing click>=8.1.3 should work well and hold up.

Let me know what you prefer and I'll fix it up. Thanks!

[mostaphaRoudsari]

Thank you, @noam-ronen for the PR.

@chriswmackey, I think the risks of changing it outweigh the benefits at this point. We don't use black for auto formatting and I don't think we need to do it any time soon.

Changing the version of click in ladybug which is very low in the dependency tree can start creating conflicts in other libraries if they haven't been careful about setting the version for click.

I think we need a better reason to take this risk. Let's say there is a new feature in click that we would like to use, etc.

[chriswmackey]

Thanks, @mostaphaRoudsari ,

I have tried to be diligent about making this requirements.txt file here in ladybug-core the one place that sets the click dependency for the whole set of Ladybug Tools core libraries (since practically all of them depend on this one).

But there are a few places where we have the click dependency specified again (eg. in the UWG repo, which is its own package and not dependent on Ladybug Tools). I'm sure you also have a lot of these in pollination so the change here would have to accommodate those cases (relaxing the == operator) or those repos would have to be coordinated with the change here.

But I have been thinking to relax our strict use of == to specify the click dependency since this has created some dependency conflicts with other packages that also use click (notably the several python packages that are being developed for URBANopt like the goejson-modelica-translator that has it's click dependency specified here).

Granted, I understand delaying the change until we can get a full account of the impacts. But I sense that we may not be able to avoid this for too much longer. At least while we're using other Python packages that also rely on click.

@noam-ronen . We don't use black as Mostatpha said but do you know if they had a particular reason why black does not permit the use of click version 7? It would just be helpful to know if we can expect other packages to drop click 7 support any time soon.

[noam-ronen]

@chriswmackey The full release notes are pretty extensive , but the main reason is detailed in this issue on the black repo - basically, there was an issue with access to an internal module, which click patched right away, and black locked to 8.0.0+.

I think it makes sense to address this as click 7.1.2 was released almost 3 years ago. Please let me know if I can help to test/validate in any way 🙏

[chriswmackey]

Hey @noam-ronen ,

Sorry that it has been a while. We have decided to start testing the libraries in newer versions of Python, mostly to support compatibility with several Python packages that NREL has been publishing.

In accordance with this, I have updated our requirements to use click 8.1.3 only when the installed version of Python is 8 or greater. This will ensure that nothing breaks on our existing distributions using Python 3.7. So just make sure you are using a version of Python after 3.7 and you will be able to install click 8.1.3 alongside it. You can see the change here: https://github.com/ladybug-tools/ladybug/blob/master/requirements.txt

So I'm closing out this issue now that this has been addressed.

[noam-ronen]

Appreciate this! We can now go back to using Ladybug alongside Black in a single environment.