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

upkeep: in abcd_demo, rename abcd_timestamp to ald_timestamp #372

Closed
jdhoffa opened this issue Apr 17, 2024 · 2 comments · Fixed by #374
Closed

upkeep: in abcd_demo, rename abcd_timestamp to ald_timestamp #372

jdhoffa opened this issue Apr 17, 2024 · 2 comments · Fixed by #374
Labels
upkeep maintenance, infrastructure, and similar

Comments

@jdhoffa
Copy link
Member

jdhoffa commented Apr 17, 2024

In #315 , the abcd_demo dataset had the ald_timestamp column renamed to abcd_timestamp.

Unfortunately, the abcd data as Asset Impact distributes it still contains the name ald_timestamp.

When pressed on this topic, Asset Impact was worried the breaking change of changing a column name might impact users existing workflows, and were hesitant to do so.

We should align the column name with what is in the expected dataset.

Alternatively, we could remove the column entirely (relates to #257 ), as I don't believe it is a critical column in any of the pacta.* or r2dii.* code:
https://github.com/search?q=org%3ARMI-PACTA%20ald_timestamp&type=code
https://github.com/search?q=org%3ARMI-PACTA+abcd_timestamp&type=code

FYI @jacobvjk

@jdhoffa jdhoffa added the upkeep maintenance, infrastructure, and similar label Apr 17, 2024
@jacobvjk
Copy link
Member

I would almost opt to go for #257 and remove all the variables you identified as not crucial @jdhoffa. I think it is fair explain that other variables can be added and what for. e.g. intermediate parent for the unlikely case that someone has this kind of ownership data. Or project finance flag for.. well what exactly? In the end, this is where it can be good to link to documentation explaining the new variable grouping of pacta.multi.loanbook.*

@jdhoffa
Copy link
Member Author

jdhoffa commented Apr 18, 2024

I back it! Go for it

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
upkeep maintenance, infrastructure, and similar
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants