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.

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

Add start and end boundaries to activity list #16

Closed
wants to merge 5 commits into from

Conversation

lgangitano
Copy link

Hi,

I've added a couple of cmdline options to limit the range of dates included in reports. This way, strava_gear can be used to build yearly statistics or tracking usage since, for example, rewaxing a chain.

I've added the filter to the apply_rule function, another approach could have been applying the filter while loading data from sqlite or CSV.

Best,

L

@liskin
Copy link
Owner

liskin commented Apr 28, 2024

Hm, that's a slightly weird usecase I must say. strava-offline can already report yearly statistics (although not on a per-component basis, obviously), and rewaxing a chain would probably better be encoded as a separate component that appears on the bike at the date of your last rewaxing.

That being said, I'm not really opposed to adding command-line options for the date range, so if you make the tests green I'll merge it.

@lgangitano
Copy link
Author

strava-gear rule sets allow for more complex scenario as moving parts from a bike to another while still keeping track of usage.

IMHO, using a different component every time a chain is waxed makes it more difficult to track overall usage.

@liskin
Copy link
Owner

liskin commented Apr 28, 2024

IMHO, using a different component every time a chain is waxed makes it more difficult to track overall usage.

Yeah, it'd be quite noisy indeed (well, we could add support for having multiple rules files). But then if the alternative is that you need to remember the last date of waxing, you might as well just use a single component and move its date (in other words not keep the whole waxing history).

Now that I think of this, perhaps a general concept of "last component maintenance" would be useful. Components like suspension forks, chains, bottom brackets, even pedals are meant to be serviced somewhat regularly, and having strava-gear support that natively would be useful. Just need to figure out a good UX for this.

@lgangitano
Copy link
Author

I agree. Maybe just a #tag in Strava would make it? I'd happily tag the first ride after maintenance.

@liskin
Copy link
Owner

liskin commented May 12, 2024

I agree. Maybe just a #tag in Strava would make it? I'd happily tag the first ride after maintenance.

Hm, yeah… It would complicate the implementation a bit, though. :-)
Right now it's a simple merge of two sorted sequences—activities and rules effective at a given time. If we wanted hashtags in activities to influence the rules… we'd need to first go through activities and gather these special tags, put that info into rules, and then finally do through activities/rules again. I mean, it's not that bad, just not as super simple as it used to be.

But then, perhaps that can be a completely separate report? If I understand correctly, all you need is just distance/time since last #waxing, per bicycle. You could probably do it in a single SQL query if you tried hard enough. :-)

@lgangitano
Copy link
Author

lgangitano commented May 12, 2024

I know a simple SQL query would be enough, but still it would not support the eventual component usage on multiple bikes (I have two Strava bikes for indoor/outdoor use of my roadbike sharing the drivetrain , for example).

Looks like I've sorted all the issues for this to work. Would you like to merge it?

@liskin
Copy link
Owner

liskin commented May 12, 2024

I know a simple SQL query would be enough, but still it would not support the eventual component usage on multiple bikes (I have two Strava bikes for indoor/outdoor use of my roadbike sharing the drivetrain , for example).

Oh, good point.

However, strava-gear right now doesn't support assigning components to multiple bikes, so you'll struggle with this usecase anyway.

Looks like I've sorted all the issues for this to work. Would you like to merge it?

I'm taking a look. One question: why do the command-line args take a different date format than rules.yaml? If you wax your chain mid-day, after a morning ride, and then go for another evening ride, there's no way to get the correct numbers out of it. So I think it'd be better if we dropped click.DateTime and just used parse_datetime everywhere :-)

(happy to implement the above, I might make other minor changes anyway)

@lgangitano
Copy link
Author

lgangitano commented May 12, 2024

I ended using parse_datetime anyway to workout timezone between rules and cli, anyway. I think we can get rid of click.DateTime.

@liskin
Copy link
Owner

liskin commented May 12, 2024

@lgangitano
Copy link
Author

Thanks! That looks way better than mine!

@liskin
Copy link
Owner

liskin commented May 12, 2024

Okay, I'll merge tomorrow, heading to bed now.

@liskin liskin closed this in d235c90 May 13, 2024
@liskin
Copy link
Owner

liskin commented May 13, 2024

(I squashed it into one commit and adjusted the commit message a bit. Hope you don't mind—the individual commits didn't pass tests anyway…)

@lgangitano
Copy link
Author

Sure! Thanks for improving it!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants