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

Example project for kompile --post-process using pyk #2729

Merged
merged 5 commits into from
Jul 22, 2022

Conversation

tothtamas28
Copy link
Contributor

Related: #2558

@radumereuta
Copy link
Contributor

This looks great. I think we should have it committed somewhere in the K repo for future reference. Not in the root directory though. We should add it to the tests as well.
Can you also add a simple tree transformation? For example, rename anonymous variables into named anonymous variables.

You should let Amelie know about this.

@tothtamas28 tothtamas28 requested a review from radumereuta July 19, 2022 11:05
@tothtamas28 tothtamas28 marked this pull request as ready for review July 19, 2022 11:05
# virtualenv

$(VENV):
virtualenv -p python3.8 $(VENV_DIR) \
Copy link
Contributor

Choose a reason for hiding this comment

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

This line specifies the python version which is not stable on all machines.
For example, I have 3.10
It's going to be really annoying to change it to test on my local machine.
Can we do something about it?
This holds true for the test in the main pyk folder as well.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I agree that moving towards supporting 3.10 as well would make sense (considering it's the Python version Ubuntu 22.04 uses). But right now, Python 3.8 is the only supported version for pyk.

Fortunately, multiple Python versions can be installed on a given system from source without messing with the system's Python paths (make altinstall): https://docs.python.org/3/using/unix.html#building-python

Virtualenv then can pick up the specified version from its install location.

Do you consider this a reasonable workaround for now?

Copy link
Contributor

Choose a reason for hiding this comment

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

Not for regression-new.
This test suite should be for quick testing and it should work on as many machines as possible.
If you move this test to the pyk folder then it should be fine. We already have this issue there.
This way, we keep regression-new worry free. You don't have to know yet another detail.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

Base automatically changed from pyk-json-fixes to master July 20, 2022 14:47
@tothtamas28 tothtamas28 force-pushed the pyk-post-process-example branch from f4bfea1 to f301523 Compare July 20, 2022 16:23
@tothtamas28 tothtamas28 requested a review from radumereuta July 20, 2022 16:25
@tothtamas28 tothtamas28 force-pushed the pyk-post-process-example branch 2 times, most recently from d389ca2 to 4ebb160 Compare July 20, 2022 16:36
@tothtamas28 tothtamas28 force-pushed the pyk-post-process-example branch from 4ebb160 to 16d34f1 Compare July 22, 2022 07:49
Copy link
Contributor

@radumereuta radumereuta left a comment

Choose a reason for hiding this comment

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

I can't make this work on my machine.
Ubuntu 22.04 can only build python 3.8.13.
I'm approving this for now, but we should really strive to support a wider range of versions. This is too frustrating.

The code lgtm

@rv-jenkins rv-jenkins merged commit 6fb6c46 into master Jul 22, 2022
@rv-jenkins rv-jenkins deleted the pyk-post-process-example branch July 22, 2022 09:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants