Skip to content

Add billable override to nonbillable projects#275

Open
jimmysway wants to merge 1 commit intoCCI-MOC:mainfrom
jimmysway:feature-274/updating-invoicing-schema
Open

Add billable override to nonbillable projects#275
jimmysway wants to merge 1 commit intoCCI-MOC:mainfrom
jimmysway:feature-274/updating-invoicing-schema

Conversation

@jimmysway
Copy link
Copy Markdown
Contributor

@jimmysway jimmysway commented Mar 6, 2026

Load optional is_billable values from projects.yaml into the nonbillable projects dataframe and update tests for the new column. Also switch the default PI input path from pi.txt to pi.yaml.

Part 1 of #274

First requires CCI-MOC/invoicing-private#89 to be merged

@jimmysway jimmysway requested review from QuanMPhm and knikolla March 6, 2026 20:04
@jimmysway jimmysway force-pushed the feature-274/updating-invoicing-schema branch from ed13429 to dde30af Compare March 10, 2026 17:25
@jimmysway jimmysway force-pushed the feature-274/updating-invoicing-schema branch from dde30af to 5336f09 Compare March 19, 2026 15:37
Comment on lines +81 to +83
billable_override_mask = (
merged_data[invoice.NONBILLABLE_IS_BILLABLE_OVERRIDE].fillna(False).eq(True)
)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Initially, my thought was fillna() should be True, since None means the project was never specified in the nonbillable file, so the default should be the project IS billable. But now I see that there are nonbillable rules independant of the nonbillable file (membership in nonbillable clusters), so I suppose this logically makes sense. The name Override becomes slightly confusing since in the case where NONBILLABLE_IS_BILLABLE_OVERRIDE is not defined, the normal nonbillable rules is the "overriding" factor.

@knikolla @naved001 Would you have any thoughts on this?

I would remove the .fillna(False).eq(True) statements. .eq(True) is redundant, and by default in Python, bool(None) == False. It also clarifies that NONBILLABLE_IS_BILLABLE_OVERRIDE for some projects were simply not set, not that they should be overriden to False

Load optional `is_billable` values from `projects.yaml` into the
nonbillable projects dataframe and update tests for the new column.
Also switch the default PI input path from `pi.txt` to `pi.yaml`.

Closes CCI-MOC#274
@jimmysway jimmysway force-pushed the feature-274/updating-invoicing-schema branch from 5336f09 to ac65c50 Compare March 20, 2026 18:50
@QuanMPhm QuanMPhm requested a review from naved001 March 27, 2026 20:13
@QuanMPhm
Copy link
Copy Markdown
Contributor

@knikolla @naved001 I've approved the PR in invoicing-private-data that this work depend on. This should no longer be blocked

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.

2 participants