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

refactor: refactor Forth generation #710

Merged
merged 52 commits into from
Oct 28, 2022
Merged

refactor: refactor Forth generation #710

merged 52 commits into from
Oct 28, 2022

Conversation

aryan26roy
Copy link
Collaborator

No description provided.

Copy link
Member

@jpivarski jpivarski left a comment

Choose a reason for hiding this comment

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

Great! We talked a lot on Zoom, and I have some requested changes, but as the next PR.

We can merge this one when the updated tests pass.

@jpivarski
Copy link
Member

This last pull from main should fix the test (from PR #719).

@jpivarski
Copy link
Member

Yes, it was just failing due to the bug that #719 fixed. The other update is not important; go ahead and merge as-is.

@jpivarski jpivarski marked this pull request as ready for review October 28, 2022 22:35
@jpivarski
Copy link
Member

I had been assuming that this was merged a month ago. I'm going to see how easy it is to get in now. Doesn't #749 need to be applied on top of these changes? Are they really independent?

@jpivarski jpivarski enabled auto-merge (squash) October 28, 2022 22:39
@jpivarski jpivarski merged commit abd63eb into main Oct 28, 2022
@jpivarski jpivarski deleted the aryan-refactoring branch October 28, 2022 22:49
jpivarski added a commit that referenced this pull request Oct 28, 2022
jpivarski added a commit that referenced this pull request Oct 28, 2022
aryan26roy added a commit that referenced this pull request Nov 6, 2022
* rename GenHelper to ForthLevelStash and add docstring with better explanation

* add error handling for Python class generation

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* change check procedure for Forth generation

* fix identation in generated Python code

* add forth generation cancellation

* add forth generation cancellation to streamer generated classes

* Rename @aryan26roy comments to 'AwkwardForth testing'.

* make forth the default

* fix some tests

* fix more tests

* chore: remove references to Identifier (once known as Identities).

* Also remove 'uproot' parameters from Forms.

* Carefully merged #710 and #749.

* This PR requires Awkward 2.0.0rc2.

* No TVector2.

* Structured the generated code in a more readable way.

* change parameters

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* fix bug

* fix awkward form

* add streamer name

* fix type

* fix typo

* fix awkward form

* fix code generation bug

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* linting

* remove ._v2

Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
Co-authored-by: jpivarski <jpivarski@gmail.com>
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