Lucene - Core
  1. Lucene - Core
  2. LUCENE-4742

Rename SpatialPrefixTree's "Node" back to "Cell"

    Details

    • Type: Improvement Improvement
    • Status: Resolved
    • Priority: Trivial Trivial
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 4.3
    • Component/s: modules/spatial
    • Labels:
      None
    • Lucene Fields:
      New

      Description

      SpatialPrefixTree makes "Node"s which are basically a rectangular spatial region that is more colloquially referred to as a "Cell". It was named "Cell" in the first place and for whatever reason, Ryan and/or Chris renamed it as part of extracting it to a top level class from an inner class. Most comments and variable names still use the "cell" terminology. I'm working on an algorithm that keeps track of a tree of "nodes" and it has gotten confusing which kind of node I'm referring to, as each Node has one cell.

      In maybe a week or so if there isn't discussion to the contrary, I'm going to commit a rename it back to "Cell". And... while we're on this naming subject, perhaps "SpatialPrefixTree" could be named "SpatialGrid" ? FWIW the variables referring to it are always "grid".

        Activity

        Hide
        David Smiley added a comment -

        The attached patch renames all "node" terminology back to "cell" in the API and code, comments, etc. I think it deserves a mention in backwards-compatibility in CHANGES.txt.

        I'll wait until your +1 Ryan.

        Show
        David Smiley added a comment - The attached patch renames all "node" terminology back to "cell" in the API and code, comments, etc. I think it deserves a mention in backwards-compatibility in CHANGES.txt. I'll wait until your +1 Ryan.
        Hide
        Ryan McKinley added a comment -

        this is fine – the term "node" came from QuadTree structures. If it conflicts with something else then use "Cell"

        Show
        Ryan McKinley added a comment - this is fine – the term "node" came from QuadTree structures. If it conflicts with something else then use "Cell"
        Hide
        David Smiley added a comment -

        Wether it's QuadTree or whatever other tree, I don't think matters. The reason why I think "Cell" is much better is that it evokes a conceptual understanding that people grok more easily in the context of spatial, I think, and it's thus colloquially also referred to as a "cell". "node" is just too generic. So using "cell" will hopefully be easier for people to understand as they use the API. Hypothetically one might create a SpatialPrefixTree with a different number of dimensions than 2, and then "cell" is less convincing of an ideal name, but nonetheless I think that's okay.

        Show
        David Smiley added a comment - Wether it's QuadTree or whatever other tree, I don't think matters. The reason why I think "Cell" is much better is that it evokes a conceptual understanding that people grok more easily in the context of spatial, I think, and it's thus colloquially also referred to as a "cell". "node" is just too generic. So using "cell" will hopefully be easier for people to understand as they use the API. Hypothetically one might create a SpatialPrefixTree with a different number of dimensions than 2, and then "cell" is less convincing of an ideal name, but nonetheless I think that's okay.
        Hide
        David Smiley added a comment -

        Node->Cell is committed on both branches, including a short note in the API changes section of CHANGES.txt.

        I'm not sure wether to take this further and have SpatialPrefixTree be SpatialGrid. It makes sense for basically the same reason as Node->Cell, but this class is more used and thus will more likely be breaking, and it also triggers other possible renames like the prefix and prefix.tree packages, and ...PrefixTree... classes. Though it'd be nice to substitute the short 'n sweet "Grid" for the longer and compound word "PrefixTree".

        Show
        David Smiley added a comment - Node->Cell is committed on both branches, including a short note in the API changes section of CHANGES.txt. I'm not sure wether to take this further and have SpatialPrefixTree be SpatialGrid. It makes sense for basically the same reason as Node->Cell, but this class is more used and thus will more likely be breaking, and it also triggers other possible renames like the prefix and prefix.tree packages, and ...PrefixTree... classes. Though it'd be nice to substitute the short 'n sweet "Grid" for the longer and compound word "PrefixTree".
        Hide
        Shawn Heisey added a comment -

        I've got the latest branch_4x and trunk pulled up in eclipse, and the Cell class (as well as the old Node class) seems to be missing. Did you forget to svn add it?

        Show
        Shawn Heisey added a comment - I've got the latest branch_4x and trunk pulled up in eclipse, and the Cell class (as well as the old Node class) seems to be missing. Did you forget to svn add it?
        Hide
        David Smiley added a comment -

        Um; it's in source control: http://svn.apache.org/repos/asf/lucene/dev/branches/branch_4x/lucene/spatial/src/java/org/apache/lucene/spatial/prefix/tree/Cell.java
        Jenkins would have had a fit if it wasn't. I'm not sure what is amiss in your local environment.

        Show
        David Smiley added a comment - Um; it's in source control: http://svn.apache.org/repos/asf/lucene/dev/branches/branch_4x/lucene/spatial/src/java/org/apache/lucene/spatial/prefix/tree/Cell.java Jenkins would have had a fit if it wasn't. I'm not sure what is amiss in your local environment.
        Hide
        Shawn Heisey added a comment -

        I'm not sure what is amiss in your local environment.

        Sorry for the false alarm. I'm not sure what went wrong either. I did completely redo the checkout and 'ant eclipse' followed by a clean/refresh, but it didn't recover properly until I completely removed the project from eclipse, deleted the checkout, and did it all again.

        Unfortunately I had deleted everything before I thought to check the filesystem for the .java file rather than the eclipse project view, so I have no way of knowing whether it was the checkout or eclipse that was screwed up.

        Show
        Shawn Heisey added a comment - I'm not sure what is amiss in your local environment. Sorry for the false alarm. I'm not sure what went wrong either. I did completely redo the checkout and 'ant eclipse' followed by a clean/refresh, but it didn't recover properly until I completely removed the project from eclipse, deleted the checkout, and did it all again. Unfortunately I had deleted everything before I thought to check the filesystem for the .java file rather than the eclipse project view, so I have no way of knowing whether it was the checkout or eclipse that was screwed up.
        Hide
        David Smiley added a comment -

        Closing against 4.3 where it Node->Cell done.

        Show
        David Smiley added a comment - Closing against 4.3 where it Node->Cell done.

          People

          • Assignee:
            David Smiley
            Reporter:
            David Smiley
          • Votes:
            0 Vote for this issue
            Watchers:
            3 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development