Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Custom period #2192

Merged
merged 1 commit into from
Dec 30, 2024
Merged

Custom period #2192

merged 1 commit into from
Dec 30, 2024
+14 −17

Conversation

R5dan
Copy link
Contributor

@R5dan R5dan commented Dec 24, 2024

Fix for #1972.

Coverts custom periods into a start time (with end being today)

@ValueRaider
Copy link
Collaborator

Merge logic into utils._interval_to_timedelta

@R5dan
Copy link
Contributor Author

R5dan commented Dec 29, 2024

@ValueRaider From what I can see it would only be called then if the interval is 5d, 1wk, 1mo, 3mo: where the original code for utils._interval_to_timedelta is fine. It would also then only be run if repair is True. Could you explain why you want these changes?

@ValueRaider
Copy link
Collaborator

It's good coding practice: avoid duplicate logic.

It would also then only be run if repair is True

Huh? You are allowed to add another call to _interval_to_timedelta.

@R5dan
Copy link
Contributor Author

R5dan commented Dec 30, 2024

@ValueRaider done

@ValueRaider ValueRaider merged commit a928eb4 into ranaroussi:dev Dec 30, 2024
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@R5dan @ValueRaider