Unconsistency in week Date filter

[mailhot001]

The week filter is not consistent, there is a 1 week offset when appling a date filter.
For example if you rund the DEMO.

Show the Expenses, and apply the daily interval, with the changes(weekly) display, the expenses are shown per week, as expected.

If you click on the 2017-W36 week, there will be no data shown, to show the 2017-W36 expenses you need to filter the date to 2017-W35.

I believe the proper week is 2017-W36 so there is a -1 week difference in the week date filter?

[mailhot001]

In fava/src/fava/util/date.py
there is the following function:

There is a +1 in the date defining, so if a date is ISO week no. 1 it will return week 2.
So it seem to be a wanted behavious, but fava on the display use the standard ISO week format, thus the inconsistency?

[mailhot001]

Test did not pass, confirming this is a wanted behaviour, so maybe we should change the week number display on weekly expenses display?

[yagebu]

Test did not pass, confirming this is a wanted behaviour

That's not necessarily the case - tests are like code, they have bugs too ;) The wanted behaviour here would be for frontend and backend to match up of course. IMHO both should just match what strftime does for %W (here's how the Python docs describe that: "Week number of the year (Monday as the first day of the week) as a zero-padded decimal number. All days in a new year preceding the first Monday are considered to be in week 0.".) - the frontend does that using d3-time-format, we should change the Python side to match.

[mailhot001]

Thanks for the info, I will double check the d3 to confirm it does the same as python fromisocalendar regarding week handling and summit an updated pull request afterwards.

[mailhot001]

You can have a look at the updated #1807 let me know what you think.
I believe the python week and d3 would align perfectly with this.

Side note, in general those who cares about week numbers are mostly european using ISO format for week number, in america we rarely use week number. You can have a read with this here it makes a lot of sense. So I wonder if eventually switching week numbering to ISO could be a better solution?

[yagebu]

Thanks for the fix, I've merged your PR.

Side note, in general those who cares about week numbers are mostly european using ISO format for week number, in america we rarely use week number. You can have a read with this here it makes a lot of sense. So I wonder if eventually switching week numbering to ISO could be a better solution?

I agree that ISO week numbers would probably be a better choice (I personally rarely use week numbers so it doesn't make much a difference to me)