-
Notifications
You must be signed in to change notification settings - Fork 122
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
WhyLabsWriter refactor #1484
WhyLabsWriter refactor #1484
Conversation
python/whylogs/experimental/performance_estimation/estimation_results.py
Outdated
Show resolved
Hide resolved
python/whylogs/api/logger/experimental/logger/actor/process_rolling_logger.py
Show resolved
Hide resolved
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.
I found two examples that breaks with the changes in this PR:
- performance_estimation.ipynb
error:
AttributeError: 'WhyLabsEstimationResultWriter' object has no attribute '_org_id'
It looks like _org_id
is now under _whylabs_client._org_id
- Writing_Reference_Profiles_to_WhyLabs.ipynb
error:
❌ Failed to upload reference profile: cannot unpack non-iterable bool object Traceback (most recent call last): File "/mnt/c/Users/felip/Documents/Projects-WhyLabs/whylogs/python/whylogs/api/whylabs/session/notebook_logger.py", line 104, in notebook_session_log result = session.upload_reference_profiles(profiles) File "/mnt/c/Users/felip/Documents/Projects-WhyLabs/whylogs/python/whylogs/api/whylabs/session/session.py", line 283, in upload_reference_profiles results.append(*[id for status, id in result if status]) File "/mnt/c/Users/felip/Documents/Projects-WhyLabs/whylogs/python/whylogs/api/whylabs/session/session.py", line 283, in <listcomp> results.append(*[id for status, id in result if status]) TypeError: cannot unpack non-iterable bool object
On a superficial inspection, the cause seems to be the output of ResultSet's write()
, which used to be a list of tuples. Now the result seems to be a single tuple.
python/whylogs/api/logger/experimental/logger/actor/process_rolling_logger.py
Show resolved
Hide resolved
This one's fixed. I also found a bug where it possible to try to write an
Also fixed. |
375a49f
to
693a6b2
Compare
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.
Please summarize what parts of the public API are being removed and deprecated with this change. We have to work through how much this affects the public surface area of our API and decide on major/minor version bump before we can merge these changes.
The main breaking change is in the
becomes
The
|
Ok its not breaking examples or public API, but adding functionality and some behavior cleanup, I think we can release in 1.4.0rc0 |
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.
LGTM for 1.4.0
Description
Writable
represents output to be serialized by whylogs. It will write itself to 1 or more temporary files. AWriter
takes the temporary file(s) and sends them to their intended destination. The interface allows anyWriter
to handle anyWritable
, but in practice someWriter
s are only interested in certain types ofWritable
s.WhyLabsWriter
acts as the main entry point for sending data to WhyLabs. It makes use of a newWhyLabsClient
interface for interacting with WhyLabs REST APIs.WhyLabsWriter
has some deprecated methods that duplicateWhyLabsClient
functionality the sake of backwards compatibility.WhyLabsWriter
delegates to theWhyLabsTransactionWriter
,WhyLabsBatchWriter
, andWhyLabsReferenceWriter
classes according to theWhyLabsWriter::write()
use case. The 3 classes correspond to the 3 REST API endpoints for uploading profiles (transaction, batch/async, and reference).WhyLabsWriterBase
implements a few utility methods shared by the variousWhyLabs*Writer
classes. In particular,_prepare_view_for_upload()
handles processing required before uploading a profile (uncompounding, custom performance metric tagging)._send_writable_to_whylabs()
serializes a profile in either V0 or V1 format and uploads it to WhyLabs._upload_view()
is a convenience method that just calls_prepare_view_for_upload()
then_send_writable_to_whylabs()
.WhyLabsWriter::write()
accepts a variety of data structures representing a profile:ViewResultSet
,ProfileResultSet
,SegmentedResultSet
,DatasetProfile
,DatasetProfileView
, andSegmentedDatasetProfileView
.WhyLabsWriterBase::_get_view_of_writable()
converts all of those exceptSegmentedResultSet
to eitherDatasetProfileView
orSegmentedDatasetProfileView
, which represent a single profile/segment to be uploaded to WhyLabs. TheWhyLabs*Writer
classes generally iterate overSegmentedResultSet
uploading each segment.zip=True
argument towrite()
Changes
TODO: describe API changes
Related
zipped batch profiles