Parameter YAML Extraction from SchematicGraph#210
Conversation
abuzali-cqc
changed the title
ad-cqc
left a comment
ad-cqc
left a comment
There was a problem hiding this comment.
BTW, in my PR, I use YAML.jl as week dependency. It may worth to consider to move deserialization functionality to the corresponding package extension.
| if !haskey(defaults, type_name) | ||
| defaults[type_name] = _params_to_dict(default_parameters(typeof(comp))) | ||
| end | ||
| if comp isa AbstractCompositeComponent |
There was a problem hiding this comment.
A Julia idiomatic way would be to override _collect_defaults! with composite component argument, instead of isa usage.
| end | ||
| end | ||
|
|
||
| function _extract_node(comp::AbstractComponent, defaults::Dict) |
There was a problem hiding this comment.
How defaults used in this function?
| "type" => type_name, | ||
| "parameters" => _params_to_dict(non_default_parameters(comp)) | ||
| ) | ||
| if comp isa AbstractCompositeComponent |
There was a problem hiding this comment.
Non-idiomatic, same as the above comment.
| end | ||
| end | ||
|
|
||
| function _write_component_entry(io::IO, entry::Dict, indent::Int) |
There was a problem hiding this comment.
YAML.jl writer could be extended by overriding YAML._print methods. If you wrap an anchor and a reference in some data type you'd be able to provide custom output for them.
| _write_yaml(io, data) | ||
| end | ||
|
|
||
| function parameters_to_yaml(sch_or_graph, filename::AbstractString) |
There was a problem hiding this comment.
Please, try to reuse YAML.jl. Why do we want to re-implement YAML writer.
|
Hi Alex, thanks for getting this rolling! What would you think of coordinating with #204 so that |
| Random = "9a3f8284-a2c9-5f02-9a11-845980a1fd5c" | ||
| SpatialIndexing = "d4ead438-fe20-5cc5-a293-4fd39a41b74c" | ||
| StaticArrays = "90137ffa-7385-5640-81b9-e52037218182" | ||
| TestItemRunner = "f8b46487-2199-4994-9208-9a1283c18c0a" |
There was a problem hiding this comment.
You shouldn't have to add TestItemRunner to the package deps -- here's the note on testing workflows I've been meaning to add to a "dev docs" page:
- To run the test suite locally:
- Recommended: In VS Code with the Julia extension, with the DeviceLayout.jl folder open, open the “Testing” sidebar (beaker symbol on the left). You can run all tests or individual TestItems, and also debug or run with coverage.
- The TestItem framework also provides options for running test items from the command line; see below
- There are also other options if this doesn’t work:
- Create an environment with the necessary dependencies and run
include("runtests.jl"). This can be convenient because you can keep that Julia session running and have subsequent test runs go much more quickly.- TestEnv.jl can be very helpful here. Add it to your default Julia environment (which should contain lightweight dev tools like this and little else). Start Julia and then activate the DeviceLayout environment, so that packages in the default environment are available. Then
using TestEnv; TestEnv.activate()will activate an environment with all base DeviceLayout.jl dependencies and additional test dependencies (Test, Aqua, and TestItemRunner).
- TestEnv.jl can be very helpful here. Add it to your default Julia environment (which should contain lightweight dev tools like this and little else). Start Julia and then activate the DeviceLayout environment, so that packages in the default environment are available. Then
- Navigate to
test/coverage, activate the environment there, andinclude("coverage.jl"), which will run tests and generate coverage statistics (as well as files that can be used with the Coverage Gutters extension for VS Code to help visualize what is covered locally). (The VS Code extension can run TestItems with coverage on Julia 1.12+.) - With DeviceLayout as the active environment or a dev’d package, run
] test DeviceLayout.
- Create an environment with the necessary dependencies and run
- Recommended: In VS Code with the Julia extension, with the DeviceLayout.jl folder open, open the “Testing” sidebar (beaker symbol on the left). You can run all tests or individual TestItems, and also debug or run with coverage.
To run TestItems from the command line, make sure you have TestEnv in your default environment, then cd /path/to/DeviceLayout.jl and start julia:
using TestEnv; TestEnv.activate()
using TestItemRunner
import DeviceLayout
TestItemRunner.run_tests(pkgdir(DeviceLayout); filter=ti -> ti.name == "Backends")- Replace the
filterfunction as needed. It receives a named tuple:(filename::String, name::String, tags::Vector{Symbol}) filenameis an absolute path, not relative- Always pass a path to
run_tests-- usepkgdir(PackageName)after the package loads
via TestEnv, or use@__DIR__only if youcdto the package root first. If you don't do this, TestItemRunner will look in the parent folder for test items and potentially pick up items from other worktrees or.julia/.
There was a problem hiding this comment.
In #204, I added parameter parsing to runtests.jl script so that they passed to TestItemRunner filter function. After that, you could run subsets of tests from Pkg.test call specifying test tags, e.g. Pkg.test(test_args=["Backends"]).
There was a problem hiding this comment.
Thanks for the note! I didn't mean to push this change (I just added it for local testing) but good to know there are better ways that don't require any modifications to the toml file.
|
@gpeairs @ad-cqc I agree with Greg's point that it makes more sense to have the extraction return a ParameterSet and leave the YAML I/O entirely to to the ParameterSet methods, so I'll likely create a new PR building off of Art's branch. I agree we should use YAML.jl wherever possible, though we might need to add some extra functionality: as far as I can tell the package doesn't support writing anchors and references (only reading them), which is important for specifying component defaults. |
3 participants
No description provided.