Allow using the projects.yaml file to identify nonbillable projects
#241
Conversation
process_report/loader.py
Outdated
|
|
||
| mask = (dataframe["Start Date"] <= invoice_settings.invoice_month) & ( | ||
| invoice_settings.invoice_month <= dataframe["End Date"] | ||
| # TODO (Quan): Do we also want to specify the clusters for these timed projects? |
There was a problem hiding this comment.
There was a problem hiding this comment.
Probably best to specify clusters whenever possible.
| # TODO (Quan): If two Coldfront projects have the same name | ||
| # (maybe because they exist on different clusters), | ||
| # the `allocation_data` dict will only accurately store info for one of the projects | ||
| # Would this be a concern? |
There was a problem hiding this comment.
There was a problem hiding this comment.
is there a way to merge it with cluster info?
There was a problem hiding this comment.
Would storing the cluster name solve this?
QuanMPhm
force-pushed
the
232/migrate_nonbillable
branch
2 times, most recently
from
1b0248c to
b3a3219
Compare
Related: CCI-MOC/non-billable-projects#49 Implementation required significant code changes: - `get_nonbillable_projects()` in `loader.py` was modified to read the `projects.yaml` file - Logic to filter for nonbillable projects have been refactored into a new function `find_nonbillable_projects()` in `validate_billable_pi_processor.py` - After some discussion, it was formally agreed that `projects.yaml` will identify projects by name, but may use IDs for certain cases. This has implications explained in `find_nonbillable_projects()` - `ColdfrontFetchProcessor`, `ValidateBillablePIsProcessor`, and test cases changed accordingly
QuanMPhm
force-pushed
the
232/migrate_nonbillable
branch
from
b3a3219 to
047671c
Compare
|
@knikolla @naved001 The PR is now considered complete and ready for review. It has 2 commits, the first allows the |
| 3. Is Timed: Boolean indicating if the nonbillable status is time-bound | ||
| """ | ||
|
|
||
| def _check_time_range(timed_object) -> bool: |
There was a problem hiding this comment.
when I have a function that returns a boolean I tend to stick to a name that tells me it'll return a boolean. Something like is_within_time_range is_in_time_range.
process_report/loader.py
Outdated
| # Leveraging inherent lexicographical order of YYYY-MM strings | ||
| return ( | ||
| timed_object["start"] <= invoice_settings.invoice_month | ||
| and invoice_settings.invoice_month < timed_object["end"] |
There was a problem hiding this comment.
the older implementation has
invoice_settings.invoice_month <= dataframe["End Date"]
is the change to < from <= intentional?
There was a problem hiding this comment.
My mistake. That should not have changed.
`_validate_allocation_data` will now consider both project and cluster names when validating nonbillable projects, as opposed to just project names. This ensures that projects with identical names but from different clusters are accurately validated against the nonbillable list.
QuanMPhm
force-pushed
the
232/migrate_nonbillable
branch
from
047671c to
92c4a2f
Compare
|
@QuanMPhm Works for me |
4 participants
Closes #232. Related: https://github.com/CCI-MOC/non-billable-projects/pull/49
More details in the commit message. This is a draft since I have some questions below.