refactor: improve readability #419
Conversation
RogerHYang
commented
Mar 22, 2023
•
RogerHYang
commented
- more imperative and fewer indents; taller and skinnier
- avoid list/dict comprehension when for-loop works
- minimize functional style
| end=time_range.end, | ||
| time_index=dataframe.index, | ||
| time_start=time_range.start, | ||
| time_stop=time_range.end, |
There was a problem hiding this comment.
| time_stop=time_range.end, | |
| time_end=time_range.end, |
There was a problem hiding this comment.
stop was intended to emphasize its end exclusive-ness, matching the terminology in the range built-in
class range(start, stop, step=1)
There was a problem hiding this comment.
My 2 cents: I think end is more clear even with the note about exclusivity
There was a problem hiding this comment.
will stay with stop, given the ambiguity with end. see JavaScript below.
const result = endOfHour(new Date(2014, 8, 2, 11, 55))
//=> Tue Sep 02 2014 11:59:59.999| time_start=time_range.start, | ||
| time_stop=time_range.end, |
There was a problem hiding this comment.
| time_start=time_range.start, | |
| time_stop=time_range.end, | |
| start_time=time_range.start, | |
| end_time=time_range.end, |
to match _results in timeseries?
| StartTime: TypeAlias = datetime | ||
| EndTime: TypeAlias = datetime |
There was a problem hiding this comment.
What is the benefit of this? Honest question. The way I see it: It forces me to have more information and reference this type, vs knowing in code that the type is datetime
There was a problem hiding this comment.
probably tenuous but mostly intended to give names to the return signature (like how you can name return variables in go)
Iterator[Tuple[StartTime, EndTime, pd.Grouper]]vs
Iterator[Tuple[datetime, datetime, pd.Grouper]]| end=time_range.end, | ||
| time_index=dataframe.index, | ||
| time_start=time_range.start, | ||
| time_stop=time_range.end, |
There was a problem hiding this comment.
My 2 cents: I think end is more clear even with the note about exclusivity
