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

Consider changing the name of the index_regions dataset #39

Closed
jdhoffa opened this issue Feb 15, 2024 · 4 comments
Closed

Consider changing the name of the index_regions dataset #39

jdhoffa opened this issue Feb 15, 2024 · 4 comments
Labels
breaking change ☠️ API change likely to affect existing code documentation feature a feature request or enhancement

Comments

@jdhoffa
Copy link
Member

jdhoffa commented Feb 15, 2024

DANGEROUS

Likely must be co-ordinated across many repos. Off the top of my head, probably warrants at least looking at:
pacta.data.preparation, portfolio.allocate, and pacta.portfolio.report
and
workflow.data.preparation and workflow.transition.monitor

cc: @cjyetman any other you can think of?

@jdhoffa jdhoffa added documentation breaking change ☠️ API change likely to affect existing code feature a feature request or enhancement labels Feb 15, 2024
@jdhoffa jdhoffa self-assigned this Feb 15, 2024
@cjyetman
Copy link
Member

probably not so bad... I think it's only on the data.prep side of things these days
https://github.com/search?q=org%3ARMI-PACTA+index_regions+-is%3Aarchived&type=code

@jdhoffa
Copy link
Member Author

jdhoffa commented Feb 16, 2024

Awesome.

I flagged DANGEROUS because it smelled like it might be, and I didn't have the time/ energy to look into it more deeply XD

@jdhoffa jdhoffa removed their assignment Apr 10, 2024
@cjyetman cjyetman added ADO and removed ADO labels Apr 27, 2024
@cjyetman
Copy link
Member

@jdhoffa what would you like to rename it as?

side note: if the intent is to distinguish it from our "indices results", I would suggest changing the names of those instead because they are really "benchmark" results which currently by default contain values from ETFs that track index compositions, but that's not strictly a requirement. index_regions actually contains the regions as specified by the index.

side note: index_regions is only ever used in the data.prep process, specifically in the monster dataprep_abcd_scen_connection(). It is only saved/exported to the outputs to allow reproducibility of the data.prep process (questionably, see RMI-PACTA/workflow.data.preparation#148)

@jdhoffa
Copy link
Member Author

jdhoffa commented Apr 29, 2024

Hmm yeah actually maybe the name is fine, I don't remember why I initially opened this repo, and I didn't add enough context to the ticket. Closing now

@jdhoffa jdhoffa closed this as completed Apr 29, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking change ☠️ API change likely to affect existing code documentation feature a feature request or enhancement
Projects
None yet
Development

No branches or pull requests

2 participants