Reorganization of ObsPy submodule structure #842
Conversation
|
While thinking about this, we should also try to find a way for users to provide "external" plugins, without needing to fork the whole obspy stuff. Imagine I produce a new "acme" plugin, and I want it to be in my own python path, but I want to be able to declare it to obspy without modifying obspy's core. Something like a .obspy/enabled-plugins (ala apache2) in the user folder, where users could add extension points to obspy? |
|
maybe @ThomasLecocq - ObsPy already has a plugin system - there is no need to fork master at all |
|
@barsch woops... ok... sorry 'bout that. +1 for |
|
@ThomasLecocq Here is an example for an external waveform plugins:
The same can be done for many other things where we use the plugin system. Also +1 for Furthermore I would suggest While we are at it we could refactor In any case, I think we should wait until after the release of 0.10 which I think we should release fairly soon. The Python 3 stuff seems stable enough and we will never catch the remaining issues if we don't release. |
|
Why not using some QuakeML-style edit: or |
|
|
|
I don't know.. I like |
|
I don't like I don't have an issue with introducing how about |
too easy to misinterpret as related to "network" as in "seismic network" IMHO here's one pretty conservative reorganization possibility: |
|
I like You could use Oh, and could we please clarify the situation with the functions for calculating GC distance; there are no less than 3 of them (depending on how you count the imports). |
mseed == MiniSEED-> data only
|
|
I strongly prefer +1 for cleaning up the geodetic functions! |
|
Updated tree, more comments, votes for changes?
It's pretty different, the most important functionality in |
|
I would like to propose that modules This should also help to provide (nearly) full support for SEED. One would need to implement the few missing ASCII blockettes. The idea is to provide a fullseed reader, by dispatching ASCII blockets to the existing parser as implemented in xseed and the binary blockettes to libmseed, provide several objects, but read the whole file only once. |
|
|
In the very long run we should probably change the @petrrr I don't mind merging In regards to a full SEED reader, the following would do the trick: from obspy import read
from obspy.xseed import Parser
def read_seed(filename):
try:
st = read(filename)
except:
st = None
try:
p = Parser(filename)
except:
p = None
return st, pThe overhead is minimal compared to your approach and thus not really worth the effort. Both reading methods will just jump around the records in the files until it finds some it can parse so the file is not really read twice . The dataless SEED part is usually only a couple records at the beginning of a file. If one really cares about performance, one should not really use full SEED in any case. We furthermore currently lack an internal data structure to deal with mixed data types, e.g. waveforms and station metadata but this is something we've been thinking about for a while. |
And also a monster task. Only if it would somehow work to use the IRIS Java tool internally this would be feasible IMO. But we've seen only possibilities that still rely on the Java VM at runtime which is out of the question.. |
|
I've put the module tree sketch at the top comment, modifications can go there now.. |
|
So should we start this after we release 0.10 or before? I would prefer to do it afterwards, at least we should have merged most open PRs before we do this and maybe do it really quick in a sprint so we avoid merge hell as much as possible |
|
@krischer: Considering SEED a legacy format or not, might be less relevant from a practical point of view. Even if we might consider it technically outdated, I do not see it disappearing any soon. It is just too established -- maybe even for good reasons as it solved already may problems with different formats which existed before -- while there is a lack of a well-established alternatives which could be picked up right away. (Yes, I know about proposals). This may change over time, but such processes take time and we are not there yet. I agree, that we may not need to consider some features of SEED which are rarely used. Maybe you are right that traversing file several times is not such a big issue. Still I would consider the current implementation incomplete (the proposed solution is not fully equivalent to full SEED support) and some task are difficult to deal with (both in Obspy as well as in general). But I accept that as long as it is only me to have this issue, we can live without addressing this. |
|
What do you mean by rarely used features of SEED? One of the few things that is currently impossible with ObsPy is writing full SEED files. But that would actually be fairly easy to add in case such a thing is desired. Otherwise we also cannot really extract information from exotic data only blockettes but that again would be fairly simple to add. Regarding dataless blockettes I think we can deal with all of them, otherwise |
|
if we completely change the structure anyway we could use the opportunity to rename also I would wait for the next release and start work on it at a dedicated dev sprint |
What for? matplotlib is a hard dependency. On top of that, matplotlib is only imported in signal inside the respective routines that use it. |
|
@megies It's effectively better now, respect to #576, since matplotlib is now 'lazy' imported. And apparently, So, for the moment, that's ok for me, unless @petrrr has other comments. |
|
I find this part counter intuitive... station-> then inventory & network... |
|
I finished up the network tests and docs, though I didn't change the tutorial yet. If we go the deprecation route, those would probably be handy for testing. The docs were changed rather bluntly, so they might require some new top-level pages or the like. |
|
@megies, @krischer, @QuLogic sorry, brain fart, I think I forgot you guys are merging My point was, I don't quite see a major version bump as "just a number". Probably not a big deal for this one, but IMO there should be a clearly stated policy if say, bug fixes will not be ported to a |
I second this notion. While there should not be too much meaning packed into version numbers, changing major version numbers at major changes can be very useful. Fast forward a few years, keeping in mind production environments that do not change quickly, and it will be very easy to remember when ObsPy change to Python3 only based solely on major version number. Imagine if what we know as Python 3 was versioned Python 0.12.45.3, it would be basically useless. |
|
I think you may have misread my "just a number" comment; I meant we shouldn't be afraid to make the jump to 1.0 if the changes are major enough to warrant it (which I think means we both agree.) Of course, there are also some connotations with a "1.0" that aren't there with a "2.0", namely stability, etc., but I think (except for this proposed change), ObsPy really is quite stable at the moment. |
|
We divagated from the actual topic into a discussion of version numbers ;o). Back to the topic: I'd vote for merging this, let's resolve the remainder in master --- and the next release is not in the near future --- so there remains lot's of time to discuss.... |
|
Give me like one more day and then we can merge this. I really want the import deprecations to work and I think I can get it working. I want that my code that depends on ObsPy runs on the current master and the latest stable. Otherwise it just makes my (and I assume others as well) life harder and honestly we should not merge anything that makes things more cumbersome in the long run for us. |
Very clean and much easier to maintain way.
Also more modules part of the import deprecator.
|
Turned out easier (and cleaner) than expected :-) Sometimes Python is quite nice to use. Feel free to merge as soon as CI passes. Essentially the imports of the previous version now still work but raise a deprecation warning. |
Reorganization of ObsPy submodule structure
|
I just noticed that @krischer, what's the rationale here? Isn't that introducing kind of an inconsistency? |
As mentioned a lot of times (recently in #841 (comment)), we should think about reorganizing the internal submodule structure.
DeprectionWarnings and redirecting imports for one full major release cycle (like we did forobspy.signal.psdwhen renaming toobspy.signal.spectral_estimation; e.g. if we make the switch for 0.10.0 all imports need to be redirected until at 0.11.0).obspy.plugins?obspy.formats?obspy.io?more comments...?
current plan: