Simplify get_components

[JoranAngevaare]

What is the problem / what does the code in this PR do

• Use self.key_for in get_components instead of strax.DataKey
• add a small test. I thought something did not work well but actually, it does.

[WenzDaniel]

Use self.key_for in get_components instead of strax.DataKey

Anything else you changed? It is hard to tell with all those renaming,

[JoranAngevaare]

Use self.key_for in get_components instead of strax.DataKey

Anything else you changed? It is hard to tell with all those renaming,

Not much no, I changed that twice and added a small test. I'm half doubting if such a cleanup of code is worth changing the git history but on the other hand, it took me some time to understand the code because of these naming conventions. What do you think?

[WenzDaniel]

I think it is fine. I was just wondering if there is anything I have to pay special attention to.

[WenzDaniel]

Okay I think it is fine. Only two things could we name it target inside cache(target) instead of target_i? Inside the function there is not any loop so I think target_i makes it a bit confusing. Instead I would suggest to use target_i in lines 869 and 870.

This PR also affects multiple other PRs like #512 and #509. Which one to merge first?

One question so it seems like you would like to use name_i as the new convention when ever we loop over something. Is this correct? This is fine with me, but what would be the convention for the corresponding index in that case? e,g "ind_target"? We just should agree on something.

[JoranAngevaare]

This PR also affects multiple other PRs like #512 and #509. Which one to merge first?

sure, any deps should be easy to solve

One question so it seems like you would like to use name_i as the new convention when ever we loop over something. Is this correct? This is fine with me, but what would be the convention for the corresponding index in that case? e,g "ind_target"? We just should agree on something.

So the reason I used target_i was that targets was already in the same scope and I try avoiding using target and targets in the same scope as I think this makes for unclear code. this_target was the other option I was contemplating but it's more letters. Maybe you have another suggestion of a convention that makes sense?

[WenzDaniel]

Okay I see no, lets leave target_i then.