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

Remove disabled code. #2334

Merged
merged 1 commit into from Aug 26, 2013
Merged

Remove disabled code. #2334

merged 1 commit into from Aug 26, 2013

Conversation

andreabedini
Copy link

No description provided.

@andreabedini
Copy link
Author

Travis build failure is unrelated.

@WeatherGod
Copy link
Member

Wow, down the rabbit hole... So, I was curious why that code existed in the first place, and I still don't fully understand it, but it was related to a corresponding change in matplotlib/lib/mpl_toolkits/init.py that was done by @jswhit six years ago to make mpl_toolkits a namespace package. The funny thing is that with the recent setuptools integration for 1.3.0, I thought we needed to configure it so that mpl_toolkits would be a namespace package. So, perhaps the code in mpl_toolkits/init.py is "dead" as well and should be removed? Any thoughts Jeff?

@efiring
Copy link
Member

efiring commented Aug 23, 2013

@WeatherGod I don't know why that code was ever in matplotlib's init, given that mpl itself is not a namespace package; but since mpl_toolkits is one, the code there is not dead.

@WeatherGod
Copy link
Member

setupext.py now has the following code:

class Toolkits(OptionalPackage):
    name = "toolkits"

    def get_packages(self):
        return [
            'mpl_toolkits',
            'mpl_toolkits.mplot3d',
            'mpl_toolkits.axes_grid',
            'mpl_toolkits.axes_grid1',
            'mpl_toolkits.axisartist',
            ]

    def get_namespace_packages(self):
        return ['mpl_toolkits']

The class was added last year, but the get_namespace_packages() part was
added last month by Mike to address issues with basemap. That is the part
that activates the namespace package feature for mpl_toolkits, I believe.
However, I think Mike will be in the best position to determine if that
code is indeed useless or not.

@mdboom
Copy link
Member

mdboom commented Aug 26, 2013

Yeah -- I don't think it's needed. It was put in by Charlie Moad in Jan 2006, and John Hunter disabled it (with the if 0) in March 2007. So I'm not sure it was ever needed, except possibly by Charlie who may have been installing his own code in the matplotlib namespace (just a wild guess). So, given that we haven't had this for more than 6 years, I think it's probably safe to remove.

The mpl_toolkits.__init__.py chunk is still needed, but that's been there a long time. The corresponding change to setupext.py that I made last month was due to missing something in the rewrite of setupext.py.

mdboom added a commit that referenced this pull request Aug 26, 2013
@mdboom mdboom merged commit 57ea260 into matplotlib:master Aug 26, 2013
mdboom added a commit that referenced this pull request Aug 26, 2013
@andreabedini andreabedini deleted the patch-4 branch August 28, 2013 02:20
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

4 participants