-
Notifications
You must be signed in to change notification settings - Fork 3
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
Utils renaming #138
Utils renaming #138
Conversation
@@ -95,7 +95,7 @@ | |||
"source": [ | |||
"# with prefix\n", | |||
"\n", | |||
"print(rail.core.util_stages.ColumnMapper)" | |||
"print(from rail.utils.stage_utils.ColumnMapper)" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Doesn't look correct.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Left a comment about the goldenspike.yml file; this looks good!
@@ -1,6 +1,6 @@ | |||
This directory contains an example notebook showing a simplified version of the end-to-end functionality of RAIL. | |||
|
|||
- [goldenspike.ipynb](https://lsstdescrail.readthedocs.io/en/latest/source/other-notebooks.html#goldenspike-an-example-of-an-end-to-end-analysis-using-rail) is a notebook that chains together the functionality of the creation, estimation, and evaluation modules. | |||
- [goldenspike.ipynb](https://rail-hub.readthedocs.io/projects/rail-notebooks/en/latest/rendered/goldenspike_examples/goldenspike.html) is a notebook that chains together the functionality of the creation, estimation, and evaluation modules. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice catch!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
name: line_confusion | ||
nprocess: 1 | ||
- classname: QuantityCut | ||
module_name: rail.creation.degradation.quantityCut | ||
module_name: rail.creation.degraders.quantityCut | ||
name: quantity_cut | ||
nprocess: 1 | ||
- classname: ColumnMapper |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looking at the reference to some rail.core.utilStages here, I wonder if we shouldn't just remove this file completely. When I try to search "utilStages" in my all-rail directory, nothing shows up.
I'm worried this file is probably a little old and unmaintained, and might be more harmful than helpful if we keep it around in a broken state. Additionally, I've found that recent opinions seem to be that any yaml pipeline files will live either in rail_pipelines or some other repo designed explicitly for storing and sharing pipelines.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(And when I say here, I mean a couple lines below, in the Columm Mapper stage and a few others. Github inconveniently limits comments to code the PR touches, which, I guess, fair.)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, yeah I get that, it is my negligence for missing the core.utilStages
here, they should be tools.table_tools
. I will fix it for now but let's discuss during the tag-up about whether we still want yml in the examples here .
Problem & Solution Description (including issue #)
Code Quality
#pragma: no cover
; in the case of a bugfix, a new test that breaks as a result of the bug has been added