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

Add weeks #1146

Draft
wants to merge 13 commits into
base: master
Choose a base branch
from
Draft

Add weeks #1146

wants to merge 13 commits into from

Conversation

bonjourmauko
Copy link
Member

@bonjourmauko bonjourmauko commented Aug 1, 2022 •
edited
Loading

Fixes #966
Supersedes #979

@bonjourmauko bonjourmauko added the kind:discovery Issue requires discovery: value, ux and tech label Aug 1, 2022
@bonjourmauko bonjourmauko marked this pull request as draft August 1, 2022 22:19
@bonjourmauko bonjourmauko mentioned this pull request Aug 2, 2022
Draft
This was referenced Nov 16, 2022
Open
Closed
verbman
verbman reviewed Nov 17, 2022
View reviewed changes
Copy link

@verbman verbman left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm trying to call a variable with definition_period set to periods.DAY from a variable with a definition_period set to periods.WEEK.
I'm using the "add" argument in the format: persons("variable_name", period, "add")
It does not work and I get the error: "Unable to compute variable..... for period 2019-W05, as 2019-W05 and day are incompatible periods".

It does work if the called variable period is changed from periods.DAY to periods.WEEKDAY

@bonjourmauko
Copy link
Member Author

bonjourmauko commented Nov 18, 2022 •
edited
Loading

@benjello I seem to have constrained week/day set inputs for coherence reasons (we can add and divide weeks and Monday to Sunday but not calendar days). What do you think? Also @MaxGhenis and @nikhilwoodruff. Would it make sense to make set inputs compatible between ISO weeks and calendar days for your use cases?

@bonjourmauko
Copy link
Member Author

bonjourmauko commented Nov 18, 2022 •
edited
Loading

More generally I ask myself the question of weekdays' (for example 2022-W01-1) usefulness in practice.

@bonjourmauko bonjourmauko mentioned this pull request Dec 16, 2022
Draft
@coveralls
Copy link

coveralls commented Dec 19, 2022 •
edited
Loading

Coverage Status

coverage: 73.942% (-0.7%) from 74.609%
when pulling fbce81f on add-weeks
into 169bef1 on fix-mypy-checks.

@bonjourmauko bonjourmauko added level:advanced Requires substantial OpenFisca experience kind:perf A code change that improves performance and removed kind:discovery Issue requires discovery: value, ux and tech labels Sep 13, 2024
@bonjourmauko bonjourmauko self-assigned this Sep 13, 2024
@bonjourmauko bonjourmauko changed the base branch from master to fix-flake8-doc-styles September 15, 2024 19:15
@bonjourmauko bonjourmauko mentioned this pull request Sep 16, 2024
Merged
@bonjourmauko bonjourmauko changed the base branch from fix-flake8-doc-styles to fix-mypy-checks September 16, 2024 00:57
@bonjourmauko bonjourmauko force-pushed the fix-mypy-checks branch 2 times, most recently from 1c299cb to a694d28 Compare September 16, 2024 23:07
This was referenced Sep 16, 2024
Merged
Draft
Draft
@bonjourmauko bonjourmauko force-pushed the fix-mypy-checks branch 3 times, most recently from 0d36d1a to 5aaca86 Compare September 17, 2024 03:16
@bonjourmauko bonjourmauko added this to the Add support for weeks milestone Sep 17, 2024
@bonjourmauko bonjourmauko mentioned this pull request Sep 17, 2024
Merged
Base automatically changed from fix-mypy-checks to master September 18, 2024 11:00
This was referenced Sep 19, 2024
Merged
Merged
@bonjourmauko bonjourmauko added kind:feat A feature request, a feature deprecation and removed kind:perf A code change that improves performance labels Sep 30, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@verbman verbman verbman left review comments

At least 1 approving review is required to merge this pull request.

Assignees

@bonjourmauko bonjourmauko

Labels
kind:feat A feature request, a feature deprecation level:advanced Requires substantial OpenFisca experience
Projects
OpenFisca International
Status: Delivery
Milestone
Add support for weeks
Development

Successfully merging this pull request may close these issues.

Add support for WEEK period
3 participants
@bonjourmauko @coveralls @verbman