-
Notifications
You must be signed in to change notification settings - Fork 83
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
Simplify graph_json
output and fix bugs
#3049
Conversation
Codecov Report
@@ Coverage Diff @@
## main #3049 +/- ##
=======================================
- Coverage 99.8% 99.0% -0.7%
=======================================
Files 312 312
Lines 30232 30255 +23
=======================================
- Hits 30144 29935 -209
- Misses 88 320 +232
Continue to review full report at Codecov.
|
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.
Looks great thanks for addressing this!
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.
@eccabay Thanks for adding this!
I still prefer letting users encode the output themselves as opposed to erroring out on some pipelines but I'm ok with the implementation you have now. If we decide to always try to encode to JSON, then it may be good to incorporate @angela97lin 's custom encoder so that we can catch tricky bools or other data types. That can happen in a follow up PR though.
docs/source/release_notes.rst
Outdated
* Changes | ||
* Updated the ``Pipeline.graph_json`` function to return a dictionary of "from" and "to" edges instead of tuples :pr:`3049` |
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.
This might technically be a breaking change?
Closes #3038
Also fixes two bugs that prevented some pipelines from getting JSON serialized:
np.int64
datatype, which is the type returned byInteger
ranges for hyperparameter searches. This can be changed within theInteger()
call, but that was too many places across the code base for this PR, so I left the fix withingraph_json
itself. In the future, however, it might be better practice to move over to havingInteger
generateint
types instead ofnp.int64
.