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

Unify list of obspy dependencies (hard and soft) #1786

Open
wants to merge 7 commits into
base: master
from

Conversation

Projects
None yet
3 participants
@megies
Member

megies commented May 12, 2017

This PR proposes to finally manage the list of all ObsPy dependencies (hard and soft) in one unified place (together with all other important 'headers').

Since this file is used at install time (by setup.py) it can't have other obspy/future imports in it, so we an't use the existing base.py for this. But I think it might be cleaner this way anyway.

@megies megies added the .core label May 12, 2017

@megies megies added this to the 1.1.0 milestone May 12, 2017

@megies megies requested review from QuLogic and krischer May 12, 2017

@megies megies added the cleanup label May 12, 2017

@krischer

I'm actually a bit torn on this. I like the new separation in soft/hard dependencies. I'm not so sure I prefer moving the setup related things out of setup.py - it is currently a bit long but most things are in one place at least. On the other hand the library names for example are already determined by obspy.core.utils.

So I guess I'm not really sure what I'm trying to say here but I'd like to at least discuss this a bit.

Show outdated Hide outdated obspy/core/util/headers.py
Show outdated Hide outdated obspy/core/util/headers.py
Show outdated Hide outdated obspy/core/util/headers.py
Show outdated Hide outdated obspy/core/util/headers.py
@megies

This comment has been minimized.

Show comment
Hide comment
@megies

megies May 15, 2017

Member

I'm not so sure I prefer moving the setup related things out of setup.py - it is currently a bit long but most things are in one place at least.

I know, I'm torn too, but I don't see a different way to avoid duplication here, since setup.py contents are not available in an installation, only in the source code..

On the other hand the library names for example are already determined by obspy.core.utils.

Yeah, we already have some stuff from core/util that gets used during installation, so I think it's pretty clear where to look for things that used to be in setup.py directly.

So I guess I'm not really sure what I'm trying to say here but I'd like to at least discuss this a bit.

Sure, I definitely wanted to hear opinions/concerns about this one. But I feel it's a good thing to do, actually I think it clusters more of these "header" type stuff in one common place now.

Member

megies commented May 15, 2017

I'm not so sure I prefer moving the setup related things out of setup.py - it is currently a bit long but most things are in one place at least.

I know, I'm torn too, but I don't see a different way to avoid duplication here, since setup.py contents are not available in an installation, only in the source code..

On the other hand the library names for example are already determined by obspy.core.utils.

Yeah, we already have some stuff from core/util that gets used during installation, so I think it's pretty clear where to look for things that used to be in setup.py directly.

So I guess I'm not really sure what I'm trying to say here but I'd like to at least discuss this a bit.

Sure, I definitely wanted to hear opinions/concerns about this one. But I feel it's a good thing to do, actually I think it clusters more of these "header" type stuff in one common place now.

megies added some commits May 12, 2017

move obspy header constants to new core/util/headers.py
main reason is to finally unify dependency lists that are currently
spread across setup.py/runtests.py and various other submodule locations
@megies

This comment has been minimized.

Show comment
Hide comment
@megies

megies May 18, 2017

Member

Looking at pkg_resources documentation, we might also be able to replace version comparison logic (comparing lists of split version number) by just using pkg_resources.SetuptoolsVersion (I guess we won't move away from setuptools anytime soon)

E.g. replace (other instances of version comparison mostly at least already use list of integers provided by obspy.core.util.get_dependency_version)

from obspy.core.util import to_int_or_zero
GDAL_VERSION_SUFFICIENT = [1, 7, 3] < list(map(
    to_int_or_zero, __gdal_version__.split(".")))

by:

from pkg_resources import SetuptoolsVersion, get_distribution
GDAL_VERSION_SUFFICIENT = (
    SetuptoolsVersion('1.7.3') <
    get_distribution('gdal').parsed_version)

That way, all in-between version numbers (post/pre-release) etc would be taken into account properly by just handing over version comparison logic to setuptools.

Member

megies commented May 18, 2017

Looking at pkg_resources documentation, we might also be able to replace version comparison logic (comparing lists of split version number) by just using pkg_resources.SetuptoolsVersion (I guess we won't move away from setuptools anytime soon)

E.g. replace (other instances of version comparison mostly at least already use list of integers provided by obspy.core.util.get_dependency_version)

from obspy.core.util import to_int_or_zero
GDAL_VERSION_SUFFICIENT = [1, 7, 3] < list(map(
    to_int_or_zero, __gdal_version__.split(".")))

by:

from pkg_resources import SetuptoolsVersion, get_distribution
GDAL_VERSION_SUFFICIENT = (
    SetuptoolsVersion('1.7.3') <
    get_distribution('gdal').parsed_version)

That way, all in-between version numbers (post/pre-release) etc would be taken into account properly by just handing over version comparison logic to setuptools.

@QuLogic

This comment has been minimized.

Show comment
Hide comment
@QuLogic

QuLogic May 18, 2017

Member

There's also distutils.version.LooseVersion (from the stdlib); I never knew why we didn't use it.

Member

QuLogic commented May 18, 2017

There's also distutils.version.LooseVersion (from the stdlib); I never knew why we didn't use it.

@megies

This comment has been minimized.

Show comment
Hide comment
@megies

megies Jul 27, 2017

Member

Punting to 1.2.0.. otherwise we'll never even get to 1.1.0

Everybody feel free to tackle it of course..

Member

megies commented Jul 27, 2017

Punting to 1.2.0.. otherwise we'll never even get to 1.1.0

Everybody feel free to tackle it of course..

@megies megies modified the milestones: 1.2.0, 1.1.0 Jul 27, 2017

@megies megies modified the milestones: 1.2.0, 1.3.0 Apr 19, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment