-
Notifications
You must be signed in to change notification settings - Fork 130
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
Fix code to work with Fugue 0.8.7 #245
Conversation
@@ -577,8 +578,28 @@ def _deserialize( | |||
arr = [pickle.loads(r["data"]) for r in df if r["left"] == left] | |||
if len(arr) > 0: | |||
return pd.concat(arr).sort_values(schema.names).reset_index(drop=True) | |||
return pd.DataFrame( | |||
{k: pd.Series(dtype=v) for k, v in schema.pandas_dtype.items()} | |||
# The following is how to construct an empty pandas dataframe with |
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 is the change to make fugue 0.8.6 compatible
@@ -541,6 +541,7 @@ def _distributed_compare( | |||
|
|||
def _serialize(dfs: Iterable[pd.DataFrame], left: bool) -> Iterable[Dict[str, Any]]: | |||
for df in dfs: | |||
df = df.convert_dtypes() |
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 is the change to make fugue 0.8.7+ compatible
@fdosani this change will make datacompy wotk with both the latest fugue and the lower versions. |
Thank you! Taking a look through the PR now. |
@goodwanghan I'm having some issues when I was testing with
Specifically seems to be maybe some incompatibility with Pandas dataframe conversion:
Fugue 0.8.7 works prefectly fine since the GitHub Actions tests all seem to pass just fine. EDIT: confirmed If I downgrade to
|
Yes, because 0.8.6 didn't have a cap on Triad, and triad 0.9.3 no longer works with Fugue 0.8.6-. So if people pinned the versions, they should have Fugue 0.8.6 with triad 0.9.1 but if they let the versions to be flexible, both will update to the latest versions so there shouldn't be problem. Only when you manually set triad to be 0.9.3 and Fugue to be 0.8.6- there can be compatibility issues |
And yes we can just update fugue requirement to 0.8.7 |
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.
Just one change (made a suggestion). For context I set it as: "fugue<=0.8.7,>=0.8.7",
because we use edgetest which runs once a week and will bump up versions as they pass and become available. It is automatically done via actions.
Co-authored-by: Faisal <[email protected]>
@fdosani i think this is good to merge |
@goodwanghan thank you again for your help! |
Thank you so much @fdosani and I apologize for the inconvenience. |
* Fix code to work with Fugue 0.8.7 * update * update * update * Update pyproject.toml Co-authored-by: Faisal <[email protected]> --------- Co-authored-by: Faisal <[email protected]>
No description provided.