Skip to content

Commit

Permalink
Remove panics from planning logic (#399)
Browse files Browse the repository at this point in the history
* remove panics in planning

* pin polars due to pola-rs/polars#11355
  • Loading branch information
jonmmease authored Sep 27, 2023
1 parent 1f1a483 commit 1ae6916
Show file tree
Hide file tree
Showing 3 changed files with 13 additions and 33 deletions.
6 changes: 3 additions & 3 deletions .github/workflows/build_test.yml
Original file line number Diff line number Diff line change
Expand Up @@ -311,7 +311,7 @@ jobs:
ls -la
python -m pip install vegafusion-*.whl
python -m pip install vegafusion_python_embed-*manylinux_2_17_x86_64*.whl
python -m pip install pytest vega-datasets polars duckdb vl-convert-python scikit-image pandas==2.0
python -m pip install pytest vega-datasets polars==0.19.3 duckdb vl-convert-python scikit-image pandas==2.0
- name: Test vegafusion
working-directory: python/vegafusion/
run: pytest
Expand Down Expand Up @@ -342,7 +342,7 @@ jobs:
ls -la
python -m pip install vegafusion-*.whl
python -m pip install vegafusion_python_embed-*macosx_10_7_x86_64.whl
python -m pip install pytest vega-datasets polars duckdb altair vl-convert-python scikit-image pandas==2.0
python -m pip install pytest vega-datasets polars==0.19.3 duckdb altair vl-convert-python scikit-image pandas==2.0
# Downgrade pyarrow to 10.0.1
python -m pip install pyarrow==10.0.1
Expand Down Expand Up @@ -380,7 +380,7 @@ jobs:
python -m pip install $vegafusion
python -m pip install $vegafusion_python_embed
python -m pip install pytest vega-datasets polars duckdb vl-convert-python scikit-image
python -m pip install pytest vega-datasets polars==0.19.3 duckdb vl-convert-python scikit-image
- name: Test vegafusion
working-directory: python/vegafusion/
run: pytest
Expand Down
12 changes: 4 additions & 8 deletions vegafusion-core/src/planning/split_domain_data.rs
Original file line number Diff line number Diff line change
Expand Up @@ -248,8 +248,7 @@ impl<'a> SplitScaleDomainVisitor<'a> {
}
]
}
))
.unwrap();
))?;

// Create new domain specification that uses the new dataset
let new_domain: ScaleDomainSpec = serde_json::from_value(serde_json::json!([
Expand All @@ -267,8 +266,7 @@ impl<'a> SplitScaleDomainVisitor<'a> {
escape_field(&new_data_name)
)
}
]))
.unwrap();
]))?;

(new_data, new_domain)
} else {
Expand Down Expand Up @@ -324,8 +322,7 @@ impl<'a> SplitScaleDomainVisitor<'a> {
}
]
}
))
.unwrap()
))?
} else {
// Will sort by the grouped field values
serde_json::from_value(serde_json::json!(
Expand All @@ -346,8 +343,7 @@ impl<'a> SplitScaleDomainVisitor<'a> {
}
]
}
))
.unwrap()
))?
})
}
}
28 changes: 6 additions & 22 deletions vegafusion-core/src/planning/stitch.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,30 +21,14 @@ pub fn stitch_specs(
keep_variables: &[ScopedVariable],
) -> Result<CommPlan> {
// Get client spec variable types
let client_defs: HashSet<_> = client_spec.definition_vars().unwrap().into_iter().collect();
let client_inputs: HashSet<_> = client_spec
.input_vars(task_scope)
.unwrap()
.into_iter()
.collect();
let client_updates: HashSet<_> = client_spec
.update_vars(task_scope)
.unwrap()
.into_iter()
.collect();
let client_defs: HashSet<_> = client_spec.definition_vars()?.into_iter().collect();
let client_inputs: HashSet<_> = client_spec.input_vars(task_scope)?.into_iter().collect();
let client_updates: HashSet<_> = client_spec.update_vars(task_scope)?.into_iter().collect();

// Get server spec variable types
let server_defs: HashSet<_> = server_spec.definition_vars().unwrap().into_iter().collect();
let server_inputs: HashSet<_> = server_spec
.input_vars(task_scope)
.unwrap()
.into_iter()
.collect();
let server_updates: HashSet<_> = server_spec
.update_vars(task_scope)
.unwrap()
.into_iter()
.collect();
let server_defs: HashSet<_> = server_spec.definition_vars()?.into_iter().collect();
let server_inputs: HashSet<_> = server_spec.input_vars(task_scope)?.into_iter().collect();
let server_updates: HashSet<_> = server_spec.update_vars(task_scope)?.into_iter().collect();

// Determine communication requirements
let mut server_to_client: HashSet<_> = client_inputs
Expand Down

1 comment on commit 1ae6916

@ritchie46
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Polars 0.19.5 is released to mitigate this.

Please sign in to comment.