matplotlib / basemap

Plot on map projections (with coastlines and political boundaries) using matplotlib
MIT License
774 stars 392 forks source link

six 1.16 is not permitted #536

Closed iamthad closed 2 years ago

iamthad commented 2 years ago

six version 1.16 was released 2021-05-05: https://github.com/benjaminp/six/releases/tag/1.16.0

The basemap package does not permit this version: https://github.com/matplotlib/basemap/blob/5339e5ecce34ccdb2f88a8e61dd4c0492acc4b86/packages/basemap/requirements.txt#L3

Is there a reason this version is incompatible, or can the requirements.txt be updated to allow it?

molinav commented 2 years ago

As far as I know, there should be no incompatibilities. I normally set upper pins just as a precaution, i.e. to be sure that a possible broken release in the dependencies is not propagated to the library (when installing).

In this case with six, I just forgot to increase the upper pin at some point, but it should be safe to increase it (six is quite reliable, e.g. it defines python_requires properly).

molinav commented 2 years ago

I was reviewing a bit further, and in fact six is only used inside the basemap.cm module to import iteritems: https://github.com/matplotlib/basemap/blob/c4805e34538d01289947940f523ce619e9704925/packages/basemap/src/mpl_toolkits/basemap/cm.py#L16

Later, this iteritems is used to loop over the contents of the colormap dictionaries stored in the same file. Based on how small these dictionaries are, I think that it could be possible to simply use dict.items below and remove the dependency on six: https://github.com/matplotlib/basemap/blob/c4805e34538d01289947940f523ce619e9704925/packages/basemap/src/mpl_toolkits/basemap/cm.py#L77

molinav commented 2 years ago

@iamthad After some minor refactoring of matplotlib.cm, the six module is not required anymore as a dependency. Therefore, in the next patch release basemap will not be limiting other packages that try to install the newer six version.