Details

    • Type: Improvement Improvement
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 5.3, 6.0
    • Component/s: None
    • Labels:
      None
    • Lucene Fields:
      New

      Description

      Geo3d is a powerful low-level geo API, recording places on the earth's surface in the index in three dimensions (as 3 separate numbers) and offering fast shape intersection/distance testing at search time.

      Karl Wright originally contributed this in LUCENE-6196, and we put it in spatial module, but I think a more natural place for it, for now anyway, is Lucene's sandbox module: it's very new, its APIs/abstractions are very much in flux (and the higher standards for abstractions in the spatial module cause disagreements: LUCENE-6578), Karl Wright and others could iterate faster on changes in sandbox, etc.

      This would also un-block issues like LUCENE-6480, allowing GeoPointField and BKD trees to also use geo3d.

      1. LUCENE.6607.patch
        824 kB
        Michael McCandless

        Issue Links

          Activity

          Hide
          Karl Wright added a comment - - edited

          Dependencies are as follows:

          • tests in the geo3d folder: only GeoPointTest has any spatial4j deps
          • main code in the geo3d folder: no dependencies
          • main code in the spatial4j folder: spatial4j deps
          • tests in spatial4j folder: spatial4j deps

          My suggestion is to move the geo3d folders to sandbox, and leave the other code right where it is, with appropriate referential changes.

          Show
          Karl Wright added a comment - - edited Dependencies are as follows: tests in the geo3d folder: only GeoPointTest has any spatial4j deps main code in the geo3d folder: no dependencies main code in the spatial4j folder: spatial4j deps tests in spatial4j folder: spatial4j deps My suggestion is to move the geo3d folders to sandbox, and leave the other code right where it is, with appropriate referential changes.
          Hide
          David Smiley added a comment -

          Mike, why in your view is it not sufficient to mark APIs that are particularly subject to change with @lucene.experimental ? IMO it is sufficient. I'm not really a fan of the sandbox module... or at least I'm cool with it if it's limited in scope to things that would otherwise go in Lucene-core because it is central and a large body of code.

          Show
          David Smiley added a comment - Mike, why in your view is it not sufficient to mark APIs that are particularly subject to change with @lucene.experimental ? IMO it is sufficient. I'm not really a fan of the sandbox module... or at least I'm cool with it if it's limited in scope to things that would otherwise go in Lucene-core because it is central and a large body of code.
          Hide
          Michael McCandless added a comment -

          Mike, why in your view is it not sufficient to mark APIs that are particularly subject to change with @lucene.experimental ?

          I think there are several compelling reasons to have large, new features in sandbox first, regardless of whether their eventual destination is core or misc or spatial or wherever.

          First, it sends a message to users that this is very new functionality, very subject to change, much more strongly than @lucene.experimental.

          Second, we are free to make drastic changes / iterations, and to have a lower bar that the API/abstractions are "correct". When I see discussions like LUCENE-6578, where the standards for contributing new features to the spatial module are too high (in my opinion), sandbox is the perfect answer: we can't and shouldn't try get all abstractions right from the get-go.

          Third, it keeps our modules/functions better separated. If geo3d were in sandbox from the start, it would not need the external dependencies in the spatial module (spatial4j).

          Forth, for this particular case, I think it's interesting to explore integration of BKD tree and GeoPointField with Geo3d, which are also already in sandbox.

          Net/net I think it's a big win if we move geo3d to sandbox.

          Show
          Michael McCandless added a comment - Mike, why in your view is it not sufficient to mark APIs that are particularly subject to change with @lucene.experimental ? I think there are several compelling reasons to have large, new features in sandbox first, regardless of whether their eventual destination is core or misc or spatial or wherever. First, it sends a message to users that this is very new functionality, very subject to change, much more strongly than @lucene.experimental. Second, we are free to make drastic changes / iterations, and to have a lower bar that the API/abstractions are "correct". When I see discussions like LUCENE-6578 , where the standards for contributing new features to the spatial module are too high (in my opinion), sandbox is the perfect answer: we can't and shouldn't try get all abstractions right from the get-go. Third, it keeps our modules/functions better separated. If geo3d were in sandbox from the start, it would not need the external dependencies in the spatial module (spatial4j). Forth, for this particular case, I think it's interesting to explore integration of BKD tree and GeoPointField with Geo3d, which are also already in sandbox. Net/net I think it's a big win if we move geo3d to sandbox.
          Hide
          Karl Wright added a comment -

          My 2 cents: I would love to get geo3d in Mike's hands so he can do BKD tree development on it. But I don't want to sacrifice the functional spatial4j integration either. I don't know whether lucene modules have the capability for cross-dependency or not. If not, maybe we have to think of another way.

          Show
          Karl Wright added a comment - My 2 cents: I would love to get geo3d in Mike's hands so he can do BKD tree development on it. But I don't want to sacrifice the functional spatial4j integration either. I don't know whether lucene modules have the capability for cross-dependency or not. If not, maybe we have to think of another way.
          Hide
          Michael McCandless added a comment -

          But I don't want to sacrifice the functional spatial4j integration either.

          The spatial module can depend on sandbox and use geo3d from there?

          Show
          Michael McCandless added a comment - But I don't want to sacrifice the functional spatial4j integration either. The spatial module can depend on sandbox and use geo3d from there?
          Hide
          Karl Wright added a comment -

          Fine; I just wasn't sure that was allowed.

          Show
          Karl Wright added a comment - Fine; I just wasn't sure that was allowed.
          Hide
          David Smiley added a comment -

          I get the "First" reason (signals subject-to-change in a bigger way). "Second" seems like the first. BTW I've used trunk in the past for this, which is another alternative.

          "Third" I disagree with; and I'm surprised to hear you say it keeps modules better separated because a sandbox module (in my view) does the opposite thing you purport it to do – it fragments them. It raises the question as to wether to put something in sandbox vs the module it actually is related to by subject area. Your rationale for the "fourth" shows that. If the spatial code stays in the spatial module, there would be no need to raise this issue!

          If geo3d were in sandbox from the start, it would not need the external dependencies in the spatial module (spatial4j).

          I fundamentally believe that if we add new code to Lucene (in whatever module), a user should be able to use that code with Lucene without developing any glue/adapters. This simply amounted to Geo3dShape.java (quite the bare minimum; see for yourself), which Karl provided early-on. Furthermore he & I re-used shape testing utilities in Spatial4j that surfaced bugs – they heavily use a randomized-testing philosophy which I hope you can appreciate. All other tests in Geo3D are standard static input/output unit tests. So this was very much worthwhile. But even putting aside how worthwhile it is (which you may contend no matter what I say), and putting aside my contention that it needed to integrate from the start, you seem to disagree with your own claim that Geo3d needed to go to the spatial module in order to use Spatial4j. You just told Karl that the spatial module could depend on sandbox.

          My vote is -0 because at least one of your reasons made some sense to me, but really I don't think our modules should be fragmented except for core. And I have a high bar for a -1 vote too.

          Separately, kind of a related aside, I'd like to state again that I don't think it makes sense for Geo3D to be here long term. It's an awesome computational geometry package that other projects (pick any information-retrieval / database / NoSQL store / anything really) could benefit greatly from. It's a gem, but a hidden gem as long as it's here. I hope we can improve it while it has a nice stay here, but that you could understand that one day it ought to have a home elsewhere (yet we'd still use it), be it on it's own or a part of another computational geometry lib.

          Show
          David Smiley added a comment - I get the "First" reason (signals subject-to-change in a bigger way). "Second" seems like the first. BTW I've used trunk in the past for this, which is another alternative. "Third" I disagree with; and I'm surprised to hear you say it keeps modules better separated because a sandbox module (in my view) does the opposite thing you purport it to do – it fragments them. It raises the question as to wether to put something in sandbox vs the module it actually is related to by subject area. Your rationale for the "fourth" shows that. If the spatial code stays in the spatial module, there would be no need to raise this issue! If geo3d were in sandbox from the start, it would not need the external dependencies in the spatial module (spatial4j). I fundamentally believe that if we add new code to Lucene (in whatever module), a user should be able to use that code with Lucene without developing any glue/adapters. This simply amounted to Geo3dShape.java (quite the bare minimum; see for yourself), which Karl provided early-on. Furthermore he & I re-used shape testing utilities in Spatial4j that surfaced bugs – they heavily use a randomized-testing philosophy which I hope you can appreciate. All other tests in Geo3D are standard static input/output unit tests. So this was very much worthwhile. But even putting aside how worthwhile it is (which you may contend no matter what I say), and putting aside my contention that it needed to integrate from the start, you seem to disagree with your own claim that Geo3d needed to go to the spatial module in order to use Spatial4j. You just told Karl that the spatial module could depend on sandbox. My vote is -0 because at least one of your reasons made some sense to me, but really I don't think our modules should be fragmented except for core. And I have a high bar for a -1 vote too. Separately, kind of a related aside, I'd like to state again that I don't think it makes sense for Geo3D to be here long term. It's an awesome computational geometry package that other projects (pick any information-retrieval / database / NoSQL store / anything really) could benefit greatly from. It's a gem, but a hidden gem as long as it's here. I hope we can improve it while it has a nice stay here, but that you could understand that one day it ought to have a home elsewhere (yet we'd still use it), be it on it's own or a part of another computational geometry lib.
          Hide
          Karl Wright added a comment -

          David Smiley: The main question is whether or not it's reasonable to develop a geo3d equivalent of GeoPointField, and subsequent BKD, in sandbox if a dependency lives in the spatial module and/or will eventually migrate away entirely. There's no question that random testing has been helpful and will continue to be helpful, so I don't think anyone is advocating removing that.

          I don't know how the dependency structure works between lucene modules; I would presume you can have one direction of dependency, but maybe not another, and certainly not both?

          Putting geo3d in sandbox, and having module spatial depend on module sandbox, would seem to work. All the module tests (including the randomizer ones) still would do their job, except against code that lives in sandbox. The spatial4j integration stays in the spatial module too. If it's just as easy to make the dependency go the other way, I frankly don't know enough about Lucene conventions to say which is better for now.

          Show
          Karl Wright added a comment - David Smiley : The main question is whether or not it's reasonable to develop a geo3d equivalent of GeoPointField, and subsequent BKD, in sandbox if a dependency lives in the spatial module and/or will eventually migrate away entirely. There's no question that random testing has been helpful and will continue to be helpful, so I don't think anyone is advocating removing that. I don't know how the dependency structure works between lucene modules; I would presume you can have one direction of dependency, but maybe not another, and certainly not both? Putting geo3d in sandbox, and having module spatial depend on module sandbox, would seem to work. All the module tests (including the randomizer ones) still would do their job, except against code that lives in sandbox. The spatial4j integration stays in the spatial module too. If it's just as easy to make the dependency go the other way, I frankly don't know enough about Lucene conventions to say which is better for now.
          Hide
          David Smiley added a comment -

          Oh I definitely welcome BKD / GeoPointField using Geo3D. No qualms there!

          I don't believe there's an issue with module inter-dependencies – at least Mike said so and I don't have a problem with that either from a policy perspective. Another possible option is to add a Spatial4j dependency on sandbox. If the sandbox is to get code from potentially any module, it stands to reason it should inherit their dependencies too. If users want to use only a piece of sandbox; it's on them to figure out what dependencies they actually need.

          Show
          David Smiley added a comment - Oh I definitely welcome BKD / GeoPointField using Geo3D. No qualms there! I don't believe there's an issue with module inter-dependencies – at least Mike said so and I don't have a problem with that either from a policy perspective. Another possible option is to add a Spatial4j dependency on sandbox. If the sandbox is to get code from potentially any module, it stands to reason it should inherit their dependencies too. If users want to use only a piece of sandbox; it's on them to figure out what dependencies they actually need.
          Hide
          Nicholas Knize added a comment -

          Except it's intended that BKD and GeoPointField will (eventually) graduate to core to provide a dependency free geo option. Won't introducing a spatial4j dependency pose a problem at that point?

          Show
          Nicholas Knize added a comment - Except it's intended that BKD and GeoPointField will (eventually) graduate to core to provide a dependency free geo option. Won't introducing a spatial4j dependency pose a problem at that point?
          Hide
          David Smiley added a comment -

          I'm not a fan of putting spatial in Lucene-core any more than I am a fan of putting a highlighter or suggester there (for example). I'd -1 that; sorry.

          Show
          David Smiley added a comment - I'm not a fan of putting spatial in Lucene-core any more than I am a fan of putting a highlighter or suggester there (for example). I'd -1 that; sorry.
          Hide
          Karl Wright added a comment - - edited

          As I understand it, the idea is to construct a field type that represents a geo3d point. In order to be able to use BKD to search for objects in geo3d shapes, you'd obviously need geo3d math.

          Nicholas has gone to some lengths to, in essence, create a minimal 2-D geo implementation sufficient to support GeoPoint field types and BKD. Mike has built the support intended for this. It seemed reasonable to do the same thing with geo3d as well.

          So either: (1) we build true geo types into core, and accept modest new core dependencies to achieve that, or (2) we add geo field types to modules/spatial. I don't know if (2) is in fact a reasonable thing to do, however.

          Show
          Karl Wright added a comment - - edited As I understand it, the idea is to construct a field type that represents a geo3d point. In order to be able to use BKD to search for objects in geo3d shapes, you'd obviously need geo3d math. Nicholas has gone to some lengths to, in essence, create a minimal 2-D geo implementation sufficient to support GeoPoint field types and BKD. Mike has built the support intended for this. It seemed reasonable to do the same thing with geo3d as well. So either: (1) we build true geo types into core, and accept modest new core dependencies to achieve that, or (2) we add geo field types to modules/spatial. I don't know if (2) is in fact a reasonable thing to do, however.
          Hide
          Nicholas Knize added a comment -

          Wait. Core is where GeoPointField started and it's intended target. To provide a native GeoPointField to the core/*.lucene.document package. It only moved to sandbox because of its immaturity and volatile API. If you look at the field type and supported queries it's no different than Numerics and NumericRange queries.

          Show
          Nicholas Knize added a comment - Wait. Core is where GeoPointField started and it's intended target. To provide a native GeoPointField to the core/*.lucene.document package. It only moved to sandbox because of its immaturity and volatile API. If you look at the field type and supported queries it's no different than Numerics and NumericRange queries.
          Hide
          Karl Wright added a comment -

          It was never clear to me why a spatial feature would automatically be targeted at core. I have been assuming that adding field types with full query support, it's necessary to be in core. If this is not in fact true, then maybe there should be more careful consideration of where all of these spatial-type fields live.

          Show
          Karl Wright added a comment - It was never clear to me why a spatial feature would automatically be targeted at core. I have been assuming that adding field types with full query support, it's necessary to be in core. If this is not in fact true, then maybe there should be more careful consideration of where all of these spatial-type fields live.
          Hide
          Nicholas Knize added a comment -

          Not sure I understand what you mean by automatically? Did you mean deliberately? It was intended to provide a dependency free, lightweight, GeoPointField type for the 90% use-case where people want simple geopoint search using nothing other than the Core library. More advanced GIS'ish capabilities? You'd bring on the spatial module, spatial4j, jts, ejml, sis, etc. it's really that simple. Not sure what the problem is having a simple geo API in core? Doesn't it just make core that much more applicable to different application use cases? It's not a highlighter, not a suggester, it's a data type. BKD is another simple type, but requires a different codec. I really want to make both of these 3D capable. Unless I'm mistaken core is to have no dependencies? If I'm wrong then this issue is moot anyway.

          Show
          Nicholas Knize added a comment - Not sure I understand what you mean by automatically? Did you mean deliberately? It was intended to provide a dependency free, lightweight, GeoPointField type for the 90% use-case where people want simple geopoint search using nothing other than the Core library. More advanced GIS'ish capabilities? You'd bring on the spatial module, spatial4j, jts, ejml, sis, etc. it's really that simple. Not sure what the problem is having a simple geo API in core? Doesn't it just make core that much more applicable to different application use cases? It's not a highlighter, not a suggester, it's a data type. BKD is another simple type, but requires a different codec. I really want to make both of these 3D capable. Unless I'm mistaken core is to have no dependencies? If I'm wrong then this issue is moot anyway.
          Hide
          Karl Wright added a comment -

          Hi Nicholas Knize,

          We agree that this is all about dependencies, right? And we agree that, for lucene core, there are certain criteria for dependencies. I'm, first of all, trying to determine what those criteria are. The reason I am asking is because I think geo3d is not eventually going to be integrated into lucene-core.jar. It will remain a separate dependency somewhere, so that other people can use just the geo functionality outside of Lucene. It will not, however, bring in expensive and massive dependencies downstream, so I guess it satisfies the criteria? Or maybe it doesn't?

          And then I began to realize that spatial4j, which modules/spatial depends upon, is also lightweight, and provides many of the same basic abilities (minus the polygons) that you've been implementing, such as rectangles that cross date line boundaries. It also has no massive required downstream dependencies. But including this as a core dependency was apparently unacceptable, so I'm trying to figure out why, precisely? Is this a legal/management issue rather than a code issue?

          Finally, I don't know whether I am correct in assuming that introducing a new field type into lucene-core has significant benefits integration-wise over creating a new field type in modules/spatial. If there isn't any huge benefit, then maybe everyone's work here, including Michael McCandless's BKD implementation, might just as well live in modules/spatial. Mike, please clarify...

          Show
          Karl Wright added a comment - Hi Nicholas Knize , We agree that this is all about dependencies, right? And we agree that, for lucene core, there are certain criteria for dependencies. I'm, first of all, trying to determine what those criteria are. The reason I am asking is because I think geo3d is not eventually going to be integrated into lucene-core.jar. It will remain a separate dependency somewhere, so that other people can use just the geo functionality outside of Lucene. It will not, however, bring in expensive and massive dependencies downstream, so I guess it satisfies the criteria? Or maybe it doesn't? And then I began to realize that spatial4j, which modules/spatial depends upon, is also lightweight, and provides many of the same basic abilities (minus the polygons) that you've been implementing, such as rectangles that cross date line boundaries. It also has no massive required downstream dependencies. But including this as a core dependency was apparently unacceptable, so I'm trying to figure out why, precisely? Is this a legal/management issue rather than a code issue? Finally, I don't know whether I am correct in assuming that introducing a new field type into lucene-core has significant benefits integration-wise over creating a new field type in modules/spatial. If there isn't any huge benefit, then maybe everyone's work here, including Michael McCandless 's BKD implementation, might just as well live in modules/spatial. Mike, please clarify...
          Hide
          Robert Muir added a comment -

          Karl: To me its about providing spatial capabilities under the apache 2.0 license.

          I don't give a fuck what spatial4j can do, its tainted with LGPL.

          Show
          Robert Muir added a comment - Karl: To me its about providing spatial capabilities under the apache 2.0 license. I don't give a fuck what spatial4j can do, its tainted with LGPL.
          Hide
          Karl Wright added a comment -

          Hmm, it looked to me like it was apache 2.0 licensed? Where's the LGPL taint? Didn't see it in the source code, at any rate...

          Show
          Karl Wright added a comment - Hmm, it looked to me like it was apache 2.0 licensed? Where's the LGPL taint? Didn't see it in the source code, at any rate...
          Hide
          Michael McCandless added a comment -

          Where's the LGPL taint?

          spatial4j (optionally?) wraps the LGPL JTS library, I think.

          Show
          Michael McCandless added a comment - Where's the LGPL taint? spatial4j (optionally?) wraps the LGPL JTS library, I think.
          Hide
          Karl Wright added a comment -

          Ah, ok. So I agree that spatial4j for the moment cannot be a core dependency, and may be too heavyweight in any case. (FWIW, earlier versions of spatial4j did not depend on JTS, so that was the disconnect.) That answers one of my questions. I take it, then, that the lucene spatial module is therefore also tarnished? And that is the reason why GeoPointField isn't going to live there?

          I'd rather not have geo3d wind up in a library that everyone is afraid to use in any case, so if everything above is correct, I'd vote strongly for moving geo3d to sandbox (for now) and eventually to its own place.

          Show
          Karl Wright added a comment - Ah, ok. So I agree that spatial4j for the moment cannot be a core dependency, and may be too heavyweight in any case. (FWIW, earlier versions of spatial4j did not depend on JTS, so that was the disconnect.) That answers one of my questions. I take it, then, that the lucene spatial module is therefore also tarnished? And that is the reason why GeoPointField isn't going to live there? I'd rather not have geo3d wind up in a library that everyone is afraid to use in any case, so if everything above is correct, I'd vote strongly for moving geo3d to sandbox (for now) and eventually to its own place.
          Hide
          Nicholas Knize added a comment -

          (I'm not a lawyer so this is added purely for discussion purposes)

          com.spatial4j.core.shape.jts.* imports com.vividsolutions.jts.geom.*

          The POM marks JTS as optional, the code comment notes:

           /** 
           *  Wraps a JTS {@link Geometry} (i.e. may be a polygon or basically anything).
           *  JTS does a great deal of the hard work, but there is work here in handling
           *  dateline wrap.
           **/
          

          And effectively uses JTS for all DE9IM spatial relation computations (which is quite expensive anyway).

          The good news is, while JTS is still currently LGPL its my understanding that Martin Davis is close to having this converted to dual EPL/BSD licensed under the LocationTech umbrella. What very little I do know about the legal issues for LGPL is from the license:

          Applications which link to LGPL libraries need not be released under the LGPL. Applications need only follow the requirements in section 6 of the LGPL: allow new versions of the library to be linked with the application; and allow reverse engineering to debug this.

          From the early incubation site: http://incubator.apache.org/ip-clearance/local-lucene-solr.html

          There is a dependency on LGPL code that will be removed before release

          Someone much more knowledgeable than me in the licensing area could provide better insight. I'm under the impression wonky licensing issues and "wrapping tricks" like this is one (of many reasons) core avoids dependencies altogether.

          Show
          Nicholas Knize added a comment - (I'm not a lawyer so this is added purely for discussion purposes) com.spatial4j.core.shape.jts.* imports com.vividsolutions.jts.geom.* The POM marks JTS as optional, the code comment notes: /** * Wraps a JTS {@link Geometry} (i.e. may be a polygon or basically anything). * JTS does a great deal of the hard work, but there is work here in handling * dateline wrap. **/ And effectively uses JTS for all DE9IM spatial relation computations (which is quite expensive anyway). The good news is, while JTS is still currently LGPL its my understanding that Martin Davis is close to having this converted to dual EPL/BSD licensed under the LocationTech umbrella. What very little I do know about the legal issues for LGPL is from the license: Applications which link to LGPL libraries need not be released under the LGPL. Applications need only follow the requirements in section 6 of the LGPL: allow new versions of the library to be linked with the application; and allow reverse engineering to debug this. From the early incubation site: http://incubator.apache.org/ip-clearance/local-lucene-solr.html There is a dependency on LGPL code that will be removed before release Someone much more knowledgeable than me in the licensing area could provide better insight. I'm under the impression wonky licensing issues and "wrapping tricks" like this is one (of many reasons) core avoids dependencies altogether.
          Hide
          Karl Wright added a comment -

          David Smiley, Michael McCandless: One proposal might be to set up a whole new module, e.g. spatial-3d, and have both sandbox and modules/spatial depend on it. Thoughts?

          Show
          Karl Wright added a comment - David Smiley , Michael McCandless : One proposal might be to set up a whole new module, e.g. spatial-3d, and have both sandbox and modules/spatial depend on it. Thoughts?
          Hide
          Michael McCandless added a comment -

          One proposal might be to set up a whole new module, e.g. spatial-3d, and have both sandbox and modules/spatial depend on it. Thoughts?

          +1, I think that's a good solution!

          Show
          Michael McCandless added a comment - One proposal might be to set up a whole new module, e.g. spatial-3d, and have both sandbox and modules/spatial depend on it. Thoughts? +1, I think that's a good solution!
          Hide
          Nicholas Knize added a comment -

          I'm not opposed that idea, I just don't know what it solves for simple Geo field types in core if core should have no dependencies? Isn't that the same as just moving Geo3d to sandbox and having the spatial module depend on sandbox? Are there 2 separate issues here?

          Show
          Nicholas Knize added a comment - I'm not opposed that idea, I just don't know what it solves for simple Geo field types in core if core should have no dependencies? Isn't that the same as just moving Geo3d to sandbox and having the spatial module depend on sandbox? Are there 2 separate issues here?
          Hide
          Karl Wright added a comment -

          I think I'm still expecting any GeoPoint3dField types to be part of modules/spatial3d. Either that, or the "no-dependencies" rule gets relaxed. But in any case I don't know enough about Lucene's rules and/or field type limitations to be able to recommend something meeting all requirements. Leaving that up to Michael McCandless

          Show
          Karl Wright added a comment - I think I'm still expecting any GeoPoint3dField types to be part of modules/spatial3d. Either that, or the "no-dependencies" rule gets relaxed. But in any case I don't know enough about Lucene's rules and/or field type limitations to be able to recommend something meeting all requirements. Leaving that up to Michael McCandless
          Hide
          Michael McCandless added a comment -

          I think I'm still expecting any GeoPoint3dField types to be part of modules/spatial3d.

          I like the separate module idea, and I like the proposed name spatial3d ... but I am no good with ant build files and pom.xml's and ivy.xml's and such, but I'll try pulling it out and see how far I get. Now seems like a good time since there are no outstanding issues/patches for geo3d?

          Show
          Michael McCandless added a comment - I think I'm still expecting any GeoPoint3dField types to be part of modules/spatial3d. I like the separate module idea, and I like the proposed name spatial3d ... but I am no good with ant build files and pom.xml's and ivy.xml's and such, but I'll try pulling it out and see how far I get. Now seems like a good time since there are no outstanding issues/patches for geo3d?
          Hide
          Karl Wright added a comment -

          Hi Mike,
          Nothing is outstanding at this time.

          Show
          Karl Wright added a comment - Hi Mike, Nothing is outstanding at this time.
          Hide
          Michael McCandless added a comment -

          Turns out this was basically trivial: I followed Karl Wright's instructions up above.

          I tried to enable javadoc linter but there are failures... we can do that separately.

          Show
          Michael McCandless added a comment - Turns out this was basically trivial: I followed Karl Wright 's instructions up above. I tried to enable javadoc linter but there are failures... we can do that separately.
          Hide
          Karl Wright added a comment -

          Looks good from what I can see!
          If you let me know what's wrong with the javadoc I can submit a patch for that.

          Show
          Karl Wright added a comment - Looks good from what I can see! If you let me know what's wrong with the javadoc I can submit a patch for that.
          Hide
          Michael McCandless added a comment -

          Thanks Karl Wright, I'll commit and open a followon issue for the javadocs...

          Show
          Michael McCandless added a comment - Thanks Karl Wright , I'll commit and open a followon issue for the javadocs...
          Hide
          ASF subversion and git services added a comment -

          Commit 1690496 from Michael McCandless in branch 'dev/trunk'
          [ https://svn.apache.org/r1690496 ]

          LUCENE-6607: factor geo3d out to its own spatial3d module

          Show
          ASF subversion and git services added a comment - Commit 1690496 from Michael McCandless in branch 'dev/trunk' [ https://svn.apache.org/r1690496 ] LUCENE-6607 : factor geo3d out to its own spatial3d module
          Hide
          ASF subversion and git services added a comment -

          Commit 1690505 from Michael McCandless in branch 'dev/branches/branch_5x'
          [ https://svn.apache.org/r1690505 ]

          LUCENE-6607: factor geo3d out to its own spatial3d module

          Show
          ASF subversion and git services added a comment - Commit 1690505 from Michael McCandless in branch 'dev/branches/branch_5x' [ https://svn.apache.org/r1690505 ] LUCENE-6607 : factor geo3d out to its own spatial3d module
          Hide
          Michael McCandless added a comment -

          I opened LUCENE-6675 for the javadocs linting issues...

          Show
          Michael McCandless added a comment - I opened LUCENE-6675 for the javadocs linting issues...
          Hide
          ASF subversion and git services added a comment -

          Commit 1690614 from Michael McCandless in branch 'dev/trunk'
          [ https://svn.apache.org/r1690614 ]

          LUCENE-6607: attempt to add pom.xml for new spatial3d module

          Show
          ASF subversion and git services added a comment - Commit 1690614 from Michael McCandless in branch 'dev/trunk' [ https://svn.apache.org/r1690614 ] LUCENE-6607 : attempt to add pom.xml for new spatial3d module
          Hide
          ASF subversion and git services added a comment -

          Commit 1690615 from Michael McCandless in branch 'dev/branches/branch_5x'
          [ https://svn.apache.org/r1690615 ]

          LUCENE-6607: attempt to add pom.xml for new spatial3d module

          Show
          ASF subversion and git services added a comment - Commit 1690615 from Michael McCandless in branch 'dev/branches/branch_5x' [ https://svn.apache.org/r1690615 ] LUCENE-6607 : attempt to add pom.xml for new spatial3d module
          Hide
          ASF subversion and git services added a comment -

          Commit 1690681 from Steve Rowe in branch 'dev/trunk'
          [ https://svn.apache.org/r1690681 ]

          LUCENE-6607: spatial3d IntelliJ config

          Show
          ASF subversion and git services added a comment - Commit 1690681 from Steve Rowe in branch 'dev/trunk' [ https://svn.apache.org/r1690681 ] LUCENE-6607 : spatial3d IntelliJ config
          Hide
          ASF subversion and git services added a comment -

          Commit 1690682 from Steve Rowe in branch 'dev/branches/branch_5x'
          [ https://svn.apache.org/r1690682 ]

          LUCENE-6607: spatial3d IntelliJ config (merged trunk r1690681)

          Show
          ASF subversion and git services added a comment - Commit 1690682 from Steve Rowe in branch 'dev/branches/branch_5x' [ https://svn.apache.org/r1690682 ] LUCENE-6607 : spatial3d IntelliJ config (merged trunk r1690681)
          Hide
          ASF subversion and git services added a comment -

          Commit 1690842 from Steve Rowe in branch 'dev/trunk'
          [ https://svn.apache.org/r1690842 ]

          LUCENE-6607: Fix spatial3d module's Maven config - include dependency interpolation sites, make packaging jar instead of pom, don't skip deploy phase, etc.; and fix GetMavenDependenciesTask to exclude a module's build artifacts only if the build dir fully matches, rather than a prefix (this bug caused lucene-spatial's test dependency on the lucene-spatial3d jar to be left out of the generated POM, because lucene/build/spatial3d matched the regex for lucene-spatial's build output dir: 'lucene/build/spatial', i.e. with no dir separator)

          Show
          ASF subversion and git services added a comment - Commit 1690842 from Steve Rowe in branch 'dev/trunk' [ https://svn.apache.org/r1690842 ] LUCENE-6607 : Fix spatial3d module's Maven config - include dependency interpolation sites, make packaging jar instead of pom, don't skip deploy phase, etc.; and fix GetMavenDependenciesTask to exclude a module's build artifacts only if the build dir fully matches, rather than a prefix (this bug caused lucene-spatial's test dependency on the lucene-spatial3d jar to be left out of the generated POM, because lucene/build/spatial3d matched the regex for lucene-spatial's build output dir: 'lucene/build/spatial', i.e. with no dir separator)
          Hide
          ASF subversion and git services added a comment -

          Commit 1690848 from Steve Rowe in branch 'dev/branches/branch_5x'
          [ https://svn.apache.org/r1690848 ]

          LUCENE-6607: Fix spatial3d module's Maven config - include dependency interpolation sites, make packaging jar instead of pom, don't skip deploy phase, etc.; and fix GetMavenDependenciesTask to exclude a module's build artifacts only if the build dir fully matches, rather than a prefix (this bug caused lucene-spatial's test dependency on the lucene-spatial3d jar to be left out of the generated POM, because lucene/build/spatial3d matched the regex for lucene-spatial's build output dir: 'lucene/build/spatial', i.e. with no dir separator) (merged trunk r1690842)

          Show
          ASF subversion and git services added a comment - Commit 1690848 from Steve Rowe in branch 'dev/branches/branch_5x' [ https://svn.apache.org/r1690848 ] LUCENE-6607 : Fix spatial3d module's Maven config - include dependency interpolation sites, make packaging jar instead of pom, don't skip deploy phase, etc.; and fix GetMavenDependenciesTask to exclude a module's build artifacts only if the build dir fully matches, rather than a prefix (this bug caused lucene-spatial's test dependency on the lucene-spatial3d jar to be left out of the generated POM, because lucene/build/spatial3d matched the regex for lucene-spatial's build output dir: 'lucene/build/spatial', i.e. with no dir separator) (merged trunk r1690842)
          Hide
          David Smiley added a comment -

          One thing I wanted to mention but is now kinda too-late is the name of this module. When people see the "3d" I think they get the wrong impression of what's here; ditto for "Geo3d" but at least "geo3d" is actually the package and name of this body of code – not spatial3d. So my recommendation would have been "spatial-geo3d" but it's not a big deal. Maybe one day the intention is to add all manner of 3d stuff here? But my impression of Geo3d is that only a few classes if that would be re-used in an un-constrainted 3d space (not constrained to the surface of a sphere or ellipsoid). When I tell people what's going on here, I feel the need to start with a disclaimer about what it isn't.

          Show
          David Smiley added a comment - One thing I wanted to mention but is now kinda too-late is the name of this module. When people see the "3d" I think they get the wrong impression of what's here; ditto for "Geo3d" but at least "geo3d" is actually the package and name of this body of code – not spatial3d. So my recommendation would have been "spatial-geo3d" but it's not a big deal. Maybe one day the intention is to add all manner of 3d stuff here? But my impression of Geo3d is that only a few classes if that would be re-used in an un-constrainted 3d space (not constrained to the surface of a sphere or ellipsoid). When I tell people what's going on here, I feel the need to start with a disclaimer about what it isn't .
          Hide
          Karl Wright added a comment -

          You are partly correct, but I think the same could be said of lucene/spatial, in that most of the classes (and indeed most of the code) is really intended for world-surface. It's not really clear what 3d applications one would want to use with Lucene, either, but let's say this at least: geo3d classes Vector, Plane, and SidedPlane are available for general 3d usage, and right now I can't think of other shapes we'd want to throw in there, but nothing stops us from doing that. Maybe we should move those classes to a base package, to make it clear? It's only clarified by naming right now.

          So my opinion: spatial3d is actually a pretty reasonable name, and is comparable to spatial in most ways...

          Show
          Karl Wright added a comment - You are partly correct, but I think the same could be said of lucene/spatial, in that most of the classes (and indeed most of the code) is really intended for world-surface. It's not really clear what 3d applications one would want to use with Lucene, either, but let's say this at least: geo3d classes Vector, Plane, and SidedPlane are available for general 3d usage, and right now I can't think of other shapes we'd want to throw in there, but nothing stops us from doing that. Maybe we should move those classes to a base package, to make it clear? It's only clarified by naming right now. So my opinion: spatial3d is actually a pretty reasonable name, and is comparable to spatial in most ways...
          Hide
          Michael McCandless added a comment -

          Phew, thanks for fixing the shadow Maven build Steve Rowe!

          Show
          Michael McCandless added a comment - Phew, thanks for fixing the shadow Maven build Steve Rowe !
          Hide
          Karl Wright added a comment -

          Thanks to everyone for getting this done!
          And now I hope Mike has some fun with this.

          Show
          Karl Wright added a comment - Thanks to everyone for getting this done! And now I hope Mike has some fun with this.
          Hide
          Shalin Shekhar Mangar added a comment -

          Bulk close for 5.3.0 release

          Show
          Shalin Shekhar Mangar added a comment - Bulk close for 5.3.0 release

            People

            • Assignee:
              Michael McCandless
              Reporter:
              Michael McCandless
            • Votes:
              0 Vote for this issue
              Watchers:
              6 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development