Skip to content
This repository was archived by the owner on Mar 13, 2026. It is now read-only.

Squash dict add test tags#145

Open
Juan-M-V wants to merge 27 commits intomainfrom
squash-dict-add-test-tags
Open

Squash dict add test tags#145
Juan-M-V wants to merge 27 commits intomainfrom
squash-dict-add-test-tags

Conversation

@Juan-M-V
Copy link
Copy Markdown
Contributor

@Juan-M-V Juan-M-V commented Nov 16, 2022

Depends on #118

Add missing TEST comment to some hints in squash dict.

@codecov-commenter
Copy link
Copy Markdown

Codecov Report

Merging #145 (e161553) into main (b4674ea) will decrease coverage by 6.51%.
The diff coverage is 10.00%.

@@            Coverage Diff             @@
##             main     #145      +/-   ##
==========================================
- Coverage   73.23%   66.71%   -6.52%     
==========================================
  Files          11       12       +1     
  Lines         609      679      +70     
==========================================
+ Hits          446      453       +7     
- Misses        163      226      +63     
Impacted Files Coverage Δ
src/lib.rs 0.00% <ø> (ø)
src/memory_segments.rs 96.96% <ø> (ø)
src/relocatable.rs 94.28% <0.00%> (-5.72%) ⬇️
src/dict_manager.rs 3.70% <3.70%> (ø)
src/ids.rs 33.59% <22.22%> (-0.87%) ⬇️
src/cairo_runner.rs 72.56% <100.00%> (+0.36%) ⬆️

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

Comment thread src/ids.rs Outdated
fmoletta
fmoletta previously approved these changes Nov 28, 2022
Comment thread src/cairo_runner.rs
Comment on lines +103 to +106
self.hint_locals.insert(
"__dict_manager".to_string(),
PyDictManager::new().into_py(py),
);
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Let's just take the GIL here rather than run everything with it taken.

Comment thread src/dict_manager.rs
Comment on lines +144 to +145
key: PyMaybeRelocatable,
val: PyMaybeRelocatable,
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Why not always a BigInt here?

Comment thread src/ids.rs
pub(crate) struct PyTypedId {
vm: Rc<RefCell<VirtualMachine>>,
hint_value: Relocatable,
pub hint_value: Relocatable,
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

pub(crate)?

Comment thread src/memory_segments.rs
vm: Rc<RefCell<VirtualMachine>>,
pub(crate) vm: Rc<RefCell<VirtualMachine>>,
#[pyo3(get)]
memory: PyMemory,
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

It's got the VM already, do we really need to keep this instance around?

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants