Iss632 after rebase on future #721
Conversation
|
OS = |
|
OS:ubuntu-20.04 |
| f"InputOutputInformation for {format_arg_chars(command)} not provided so considered side-effectful." | ||
| ) | ||
| if io_info.has_other_outputs(): | ||
| raise Exception( | ||
| raise UnparallelizableError( |
There was a problem hiding this comment.
Konstantinos (from old branch): Maybe also print what is the other output?
There was a problem hiding this comment.
I think I may need some help with determining what the other outputs to pass it along in the exception. I see that in io_info.has_other_outputs(), it is going through a for-loop checking if each entry is a "other output" has_other_outputs() function (line 182); I'm not sure if there's already a function in the class that returns these "other outputs", but if not, would it be reasonable to change this function or add another function like this to return a list of those outputs in the class?
There was a problem hiding this comment.
I am not sure if there is such a function, but if not, you could certainly make a new one that returns a list of those outputs!
|
OS:ubuntu-20.04 |
|
OS = |
There was a problem hiding this comment.
Everything looks good other than the changes to expand.py. These changes need to happen as a new PR to that repo instead (https://github.com/binpash/sh-expand).
python_pkgs/sh_expand/expand.py
Outdated
| @@ -0,0 +1,518 @@ | |||
| import copy | |||
There was a problem hiding this comment.
This is a different package (https://github.com/binpash/sh-expand)! and the changes in this file need to happen in a PR there.
There was a problem hiding this comment.
Would you recommend for this:
- keeping the ExpansionError in the custom_error and importing it here, or
- moving the ExpansionError here and importing it to pash_compiler to catch it?
I'm not sure why, but it also seems like I don't have permission to push my branch onto this package repo.
Thanks!
There was a problem hiding this comment.
Would you recommend for this:
1. keeping the ExpansionError in the custom_error and importing it here, or 2. moving the ExpansionError here and importing it to pash_compiler to catch it?
I would probably define ExpansionError in sh-expand, and then import it in pash to catch it. It makes sense to hide all expansion related behavior and details in that package.
I'm not sure why, but it also seems like I don't have permission to push my branch onto this package repo.
That should be fixed now, could you try again? :)
There was a problem hiding this comment.
As you suggested, I imported ExpansionError into pash_compiler and defined ExpansionError in sh_expand in the original package (with a new PR)!
I think currently the tests on github actions are not passing due to ExpansionError not being found in the package yet. I've ran the tests locally with the most recent changes, and it seems to work as before so hopefully after the change in the package the issue of the ExpansionError not found in the import will not be a problem.
Thank you!
There was a problem hiding this comment.
I merged that PR (after bumping the project version because that needs to happen for the package to be properly deployed to PyPI).
Now the tests seem to be passing, so the only thing you need to do is sign off your commits (follow the instructions here) https://github.com/binpash/pash/pull/721/checks?check_run_id=26107013047 and then we can properly approve and merge!
Great work :)
|
OS = |
|
OS:ubuntu-20.04 |
|
OS = |
|
OS = |
|
OS:ubuntu-20.04 |
|
OS:ubuntu-20.04 |
|
OS = |
|
OS:ubuntu-20.04 |
|
OS:ubuntu-20.04 |
…lizableError and AdjLineNoteImplemented Error) caught before general errors, introduce custom error to ast_to_ir and ir in compiler at appropriate places with more detail error messages Signed-off-by: YUUU23 <[email protected]>
…r in pash_compiler Signed-off-by: YUUU23 <[email protected]>
…expand.py file) under one ExpansionError class in custom_error to catch and log these errors separately Signed-off-by: YUUU23 <[email protected]>
Signed-off-by: YUUU23 <[email protected]>
…n expand package Signed-off-by: YUUU23 <[email protected]>
…l package Signed-off-by: YUUU23 <[email protected]>
Signed-off-by: YUUU23 <[email protected]>
|
OS = |
|
OS:ubuntu-20.04 |
I fixed the commit signing! Ready for the final steps for merging! Thank you so much!! |
|
Thank you! |
I might've accidentally messed something up while trying to rebase my old branch to the future branch instead of working off the main branch (😅) ! I apologize for this inconvenience, but here this backup branch should have all the previous debugging improvement changes before Konstantinos's comments and also the changes made from Konstantinos's suggestions.