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

Implemented processor for BU Subsidy #113

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

QuanMPhm
Copy link
Contributor

@QuanMPhm QuanMPhm commented Nov 12, 2024

Depends on #112. Closes #102. This PR consists of 2 commits, starting from Initialized processor for BU subsidy...

The first commit copies over code without changes.
The second commit makes functional changes.
Details about the commits are found in the commit messages.

balances = pi_balances

if not project_names:
project_names = ["" for _ in range(len(pi))]
Copy link
Collaborator

Choose a reason for hiding this comment

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

If you don't care about the project name, I'd still give it a name so it's clear that it's not something you missed.

btw, this is also valid syntax if you just want repeat items in a list

>>> ['some-project']*len(pi)
['some-project', 'some-project', 'some-project', 'some-project']

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@naved001 I did the list initialization in the ["" for _ in range()] form for the previous PR as well. Is it worth it to agree on your form of list initialization going forward?

Copy link
Collaborator

Choose a reason for hiding this comment

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

@QuanMPhm how we initialize the list was merely a suggestion, feel free to ignore it. However, I do like giving things some names instead of an empty string "".

Code from `BillableInvoice` and `DiscountInvoice` has been copied over
without change
The `DiscountProcessor` class has been created, to be subclassed by
processors which applies discounts, such as the New-PI Credit.
This processor class introduces an important class constant,
`IS_DISCOUNT_BY_NERC`, which detemines whether the final balance,
as opposed to the PI balance (more info below), reflects the discount
being applied.

The New-PI credit is now implemented by `NewPICreditProcessor`

During discussions about billing, it was made noted that some discounts
are not provided by the MGHPCC, but instead from other sources, such as
the BU subsidy, which is provided to BU PIs by BU. This provided
motivation for a `PI Balance` field, which would reflect how much money the
PI should be billed, as opposed to the `Balance` field, which currently
reflects how much money the MGHPCC should receive. These two fields would
not equal each other if the PI received discounts not provided by the
MGHPCC.

Implementation of `NewPICreditProcessor` and the new billing
feature required a range of changes:
- `apply_discount_on_project()` in `DiscountProcessor` has been slightly
modified, where the PI balance and MGHPCC balance is now calculated seperately.
- As `BillableInvoice` no longer performs any processing itself, the
dataframe from `NewPICreditProcessor` is now passed to all invoice objects.
- The test cases for the New-PI credit have been refactored. Test cases
for the new billing feature is not written yet. I plan to write
them when the processor for the BU Subsidy is implemented
- With the new processor, certain Processor and Invoice classes depend on
fields created by other Processors, such as the case with
`NewPICreditProcessor` and `ValidateBillablePIsProcessor`. As such, docstrings
have been added to indicate dependancies
the `DiscountInvoice` class is now removed. No invoice class will
now class `_prepare()` or `_process()`.

`BUSubsidyProcessor`, which handles processing for the BU subsidy,
sets `IS_DISCOUNT_BY_NERC` to `False` because the
subsidy is not provided by NERC. Because of this, `BU Balance` indicates
the money which BU (not the PI they are subsidizing) owes to the
MGHPCC.

The test cases for the BU Subsidy has been refactored to be
more robust and readable.
The `Project` field has been added to `invoice.py`
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.

Refactor processing for the BU Subsidy
3 participants