arborx / ArborX

Performance-portable geometric search library
BSD 3-Clause "New" or "Revised" License
184 stars 39 forks source link

Remove PrimitivesTag and PredicatesTag #1172

Closed aprokop closed 6 days ago

aprokop commented 1 month ago

Newer version of #997. Don't know if we want to rename AccessTraits to RangeTraits.

aprokop commented 3 weeks ago

Rebased to resolve conflicts.

aprokop commented 1 week ago

Rebased to resolve conflicts.

aprokop commented 1 week ago

Would you remind me whether we considered introducing a newer traits (not just a rename) and have the old access traits automatically translated to it if no specialization is provided?

I don't think so.

dalg24 commented 1 week ago

Would you remind me whether we considered introducing a newer traits (not just a rename) and have the old access traits automatically translated to it if no specialization is provided?

I don't think so.

What are your thoughts about it? I expect that is implementable and may be nice to have but I lost track of all the changes and maybe that's just overkill.

aprokop commented 6 days ago

What are your thoughts about it? I expect that is implementable and may be nice to have but I lost track of all the changes and maybe that's just overkill.

Just so that I understand, you are talking about introducing something like RangeTraits that would not have tags, using it internally, and providing specialization that would take tagged AccessTraits? I don't quite understand what you mean by "have the old access traits automatically translated to it if no specialization is provided".

dalg24 commented 6 days ago

Just so that I understand, you are talking about introducing something like RangeTraits that would not have tags, using it internally, and providing specialization that would take tagged AccessTraits?

Yes pretty much

I don't quite understand what you mean by "have the old access traits automatically translated to it if no specialization is provided".

What you said above, "providing specialization that would take tagged AccessTraits"

aprokop commented 6 days ago

Yes pretty much

I can try to implement this. So, we would advertise new traits (RangeTraits?) but users won't have to rewrite the existing ones? Wonder how much benefit there is, as we already broke backwards compatibility in the indexes constructors. We would need to do a bunch of renaming, too, from AccessValues to RangeValues and similar.

dalg24 commented 6 days ago

I can try to implement this. So, we would advertise new traits (RangeTraits?) but users won't have to rewrite the existing ones?

Correct

Wonder how much benefit there is, as we already broke backwards compatibility in the indexes constructors.

That was my question

aprokop commented 6 days ago

Wonder how much benefit there is, as we already broke backwards compatibility in the indexes constructors.

That was my question

I see little. I'd rather have users to go through upgrade once, with the major version release. In addition, I think it would give users a chance to reexamine their AccessTraits structures to see what they actually need to return now that they are not limited to points and boxes.

I do want to decide what we should call the traits. Doesn't have to be done here, though.