deprecate get_all_items_as_dict and get_all_items, add get_items_as_dicts

[philvarner]

Related Issue(s): #187

Description:

• deprecate get_all_items_as_dict and get_all_items, as these can retrieve the entire catalog without the user really knowing
• add get_items_as_dicts to allow for retrieval of unmarshalled items

PR Checklist:

• Code is formatted
• Tests pass
• Changes are added to the CHANGELOG

[codecov-commenter]

Codecov Report

Merging #206 (0abeac6) into main (d2bce11) will decrease coverage by 0.21%.
The diff coverage is 77.27%.

@@            Coverage Diff             @@
##             main     #206      +/-   ##
==========================================
- Coverage   83.93%   83.72%   -0.22%     
==========================================
  Files           9        9              
  Lines         660      676      +16     
==========================================
+ Hits          554      566      +12     
- Misses        106      110       +4     

Impacted Files	Coverage Δ
pystac_client/client.py	80.00% <0.00%> (ø)
pystac_client/item_search.py	91.93% <80.00%> (-0.93%)	⬇️
pystac_client/collection_client.py	76.74% <100.00%> (ø)

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update d2bce11...0abeac6. Read the comment docs.

[gadomski]

One top-level thought -- in Rust, the convention is to use iter_* for methods that return iterators. Here, this would mean that get_itemcollections would be renamed to iter_itemcollections (same with iter_items). Does that scan any better to you @philvarner? Just a thought as I was musing about our discussion on whether to include all in the method name.

[philvarner]

One top-level thought -- in Rust, the convention is to use iter_* for methods that return iterators. Here, this would mean that get_itemcollections would be renamed to iter_itemcollections (same with iter_items). Does that scan any better to you @philvarner? Just a thought as I was musing about our discussion on whether to include all in the method name.

Few thoughts. I like that convention, but it's not one used in Python, so I'd avoid it. Though, I also hate methods prefixed with "get" since that's the old (entirely unncecessary) OO convention for accessor methods. I also don't like using get when the underlying method has side-effects and isn't just returning a value. I usually prefer just to name it like itemcollections() or items() or items_as_dicts(). Since they are causing network requests, I might prefix them with retrieve to indicate that there's a lot going on under the hood.

[gadomski]

it's not one used in Python, so I'd avoid it

Yup fair.

Though, I also hate methods prefixed with "get" since that's the old (entirely unncecessary) OO convention for accessor methods. I also don't like using get when the underlying method has side-effects and isn't just returning a value.

Agreed.

Since they are causing network requests, I might prefix them with retrieve to indicate that there's a lot going on under the hood.

Agreed, though since they can be GET requests I guess get_ isn't a terrible prefix either :-). Full circle.

I don't think I feel strongly enough to think a change is necessary now, but happy to support a change too with others' input.

[philvarner]

Since they are causing network requests, I might prefix them with retrieve to indicate that there's a lot going on under the hood.

Agreed, though since they can be GET requests I guess get_ isn't a terrible prefix either :-). Full circle.

Well, we default to POST, so... 🤣

I don't think I feel strongly enough to think a change is necessary now, but happy to support a change too with others' input.

oh, so apparently the get prefix was added by in v0.2.0, from the changelog:

- `Search.item_collections()` renamed to `Search.get_item_collections()`
- `Search.item_collections()` renamed to `Search.get_items()`

(though i think the second msg should say Search.items() renamed

@matthewhanson any thoughts on this?