Skip to content
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

changing wellen to a cargo workspace #17

Merged
merged 4 commits into from
May 21, 2024
Merged

Conversation

1024bees
Copy link
Contributor

Hey, we talked at latch-up -- I'd like to add python bindings to wellen.

To do this practically, wellen needs to be structured as a cargo workspace, so the python bindings and the rust library can be decoupled -- took a non-invasive, first stab at this in this commit.

Copy link
Owner

@ekiwi ekiwi left a comment

Choose a reason for hiding this comment

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

This looks generally good. Thank you very much!

I do like to keep my directory hierarchy as flat as possible. Could we get rid of the crates directory and put the new wellen folder directly into the repo root?

@1024bees
Copy link
Contributor Author

sgtm, i've moved it up to repo root

Copy link
Owner

@ekiwi ekiwi left a comment

Choose a reason for hiding this comment

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

Thanks! I will merge this as soon as the CI passes.

@ekiwi
Copy link
Owner

ekiwi commented May 20, 2024

@1024bees : Please have a look at the CI failures and try to address them. I think this is mostly about some paths that need to be fixed.

Also, please try to address these warnings:

warning: /home/runner/work/wellen/wellen/wellen/Cargo.toml: unused manifest key: package.default-members
warning: /home/runner/work/wellen/wellen/Cargo.toml: unused manifest key: workspace.package.default-members

@1024bees
Copy link
Contributor Author

1024bees commented May 20, 2024

woops, sorry about that, fixed the warnings and CI should be working.

With the way workspaces have been implemented (virtual workspaces would be best, IMO), for examples to live in the workspace root, each example needs it's own Cargo.toml file. More context provided at rust-lang/cargo#7467. An example of "expected" examples structure is here: https://github.com/tracel-ai/burn/tree/main/examples

I can do this if you'd prefer -- it might be the better long term structure for examples, but I'm indifferent to it right now. To keep friction low for this MR, examples have been moved into the wellen crate and inputs/ have been symlinked from the workspace root. running examples should be exactly the same as before this MR, if you cd into the wellen crate.

wellen/inputs Outdated Show resolved Hide resolved
@ekiwi ekiwi merged commit aaebcf9 into ekiwi:main May 21, 2024
12 checks passed
@ekiwi
Copy link
Owner

ekiwi commented Sep 18, 2024

@1024bees Have you had time to look into making Python bindings for wellen? I would still be truly interested in making wellen available to python developers because - as you pointed out - there aren't really any good VCD libraries for Python and essentially none for FST or GHW.

@1024bees
Copy link
Contributor Author

Not yet, I've been a little distracted in life, but I'd like to this and this is good motivation :). I'll look to have a draft up by ~Sunday

@ekiwi
Copy link
Owner

ekiwi commented Sep 18, 2024

Not yet, I've been a little distracted in life, but I'd like to this and this is good motivation :). I'll look to have a draft up by ~Sunday

That would be exciting. It is nice to know that you are still working on it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants