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

Invoices no longer assign to self.data during filtering step #119

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

Conversation

QuanMPhm
Copy link
Contributor

@QuanMPhm QuanMPhm commented Dec 4, 2024

Closes #111. All invoice classes will now assign to self.export_data

This removes ambiguity on whether the invoices will modify their internal dataframe and the need to call copy() on the processed dataframe

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`
Instead, all invoice classes will now assign to `self.export_data`
This removes ambiguity on whether the invoices will modify their
internal dataframe and the need to call `copy()` on the processed
dataframe
Copy link
Collaborator

@naved001 naved001 left a comment

Choose a reason for hiding this comment

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

This looks fine to me but it broke your tests.

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.

Invoice classes should not modify or reassign self.data
2 participants