HopkinsIDD/flepiMoP

Document, Test, Repair `gempyor.parameters` Module

TimothyWillard opened this issue · 3 comments

TimothyWillard commented

Very similar to GH-246 but for the gempyor.parameters module. I'm starting here since this seems to be one of the simpler modules that interact with the configuration files directly. I also suspect that I'll find some bugs/unexpected behaviors along the way (such as the time series data frame being dropped issue I've already discovered) so I'll leave open the possibility of fixing that here. If the repairs get too large in size I'll migrate that to a separate issue.

TimothyWillard commented

Methods that need to be removed on the next PR for this issue:

  1. picklable_lamda_alpha
  2. picklable_lamda_sigma
  3. get_pnames2pindex

See #277 (comment).

TimothyWillard commented

This was originally a comment in GH-277 in relation to the second insufficient dates ValueError in the constructor of the Parameters class. I'm not sure how to get to the second pathway to this error message.

  1. We subset the read in dataframe to ti to tf so if the dataframe goes from 2024-01-01 through 2024-01-05 and the given date range is 2024-01-02 through 2024-01-06 the dataframe's date range will be subsetted to 2024-01-02 through 2024-01-05 which is a repeat of the above.
  2. Because of the subsetting you can't provide anything except a monotonic increasing sequence of dates, pandas only allows subsetting on ordered date indexes so you'll get a different error.
  3. If you provide a monotonic increasing sequence of dates but 'reverse' ti and tf you get no errors (which I think is also bad) because the slice operation returns an empty dataframe with the right columns & index and the pd.date_range function only creates monotonic increasing sequences and 0 == 0.

If this is dead code then I think we should just remove it, but @saraloo and @jcblemai do y'all have any thoughts?

See: #277 (comment).

TimothyWillard commented

An open TODO for this issue that needs to be followed up on is adding descriptions to the examples of the new testing functions added per this comment: #277 (comment). Took care of the immediate one there, but will address others in a follow up.