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.

Sign up for GitHub

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

Jump to bottom

convert bytes into a string (subprocess.check_output return value) #2152

Closed
wants to merge 19 commits into from
Closed

convert bytes into a string (subprocess.check_output return value) #2152

wants to merge 19 commits into from

Conversation

takashikumagai
Copy link
Contributor

subprocess.check_output() in Python 3 returns bytes instead of a string, and therefore the return value needs to be converted into a string in order to call split() on it in L42. Not doing this causes a type mismatch error:

  File "/home/viridian/src/motioneye-fork/motioneye/mmalctl.py", line 42, in <genexpr>
    d = dict(p.split('=', 1) for p in support.split())
TypeError: a bytes-like object is required, not 'str'

@zagrim
Copy link
Collaborator

zagrim commented Jul 15, 2021 •
edited
Loading

Relates to issue #2158

@takashikumagai takashikumagai mentioned this pull request Jul 15, 2021
Closed
@MichaIng MichaIng changed the base branch from python3 to dev March 9, 2022 19:14
@MichaIng MichaIng added the python update python 2 > python 3 label Mar 9, 2022
herostrat and others added 15 commits March 9, 2022 20:38
@herostrat
Python3 Work: Dictionary Iterators and Unicode (#1978)
533cc76
@MichaIng
Re-add some lost CI fixes (#2316)
4da8a87 
From #2312 by @cclauss.

Signed-off-by: MichaIng <micha@dietpi.com>
@jmichault
changes in dockerto take into account internationalization
974a520
@jmichault
Merge branch 'dev' of https://github.com/motioneye-project/motioneye …
bde3b16 
…into dev
@jmichault
Bug Fix for Issue 2292
c60b64a
@cclauss
Travis CI last ran a year ago (#2311)
b67e78d 
https://travis-ci.org/github/ccrisan/motioneye/builds
@MichaIng
Remove legacy font formats (#2319)
866ee12 
woff is supported by all relevant browser versions: https://caniuse.com/woff

woff2 should be added as alternative with enhanced compression in a dedicated PR.

Signed-off-by: MichaIng <micha@dietpi.com>
@cclauss
Add pre-commit running bandit, codespell, flake8 (#2321)
b2be5de
@cclauss
pre-commit: Enable trim trailing whitespace (#2328)
2df0383
@cclauss
GitHub Action to test our Ubuntu wiki page (#2326)
b50c64b
@cclauss
fix end of files (#2330)
d84d2d0
@cclauss
GitHub Action to build using the steps in README.md (#2324)
72ddffb
@cclauss
Static tests are in pre-commit so focus on build, mypy, pytest, safety (
4787e41 
#2322)

* Run apt commands separately to undersstand performance

Running these commands separately delivers the same end result but provides us more visibility into where things are broken or stuck and how long each command is taking.

* python3 -m build

* https://pypi.org/project/build

* pip install .

* Delete requirements.txt

* Update and rename lint_python.yml to test_python.yml
@mercutiodesign
Fix error on entirely numeric camera_name (#1361)
c58ad0f 
If you have a camera_name that is only numbers then you would get an
AttributeError if you add multiple cameras.
@doubleO8 @MichaIng
Drop stray settings item separator (#2275)
2edd265 
Co-authored-by: MichaIng <micha@dietpi.com>
@MichaIng
Copy link
Member

@takashikumagai
Can you rebase on current dev? The mmalctl.py change has been solved already differently, but removing the buffering policy makes sense 👍.

@takashikumagai
Copy link
Contributor Author

Do you mean picking the commit that removes the buffering policy (75c68de) and rebasing only that specific commit on the dev?

@MichaIng
Copy link
Member

Jep, exactly.

@MichaIng
Copy link
Member

MichaIng commented Mar 12, 2022 •
edited
Loading

You need to rebase onto current dev, so the changes of it are not listed here a new changes 😉. Currently the PR is based on the old python3 branch.

I can also do that from here, if you want.

@MichaIng MichaIng linked an issue Mar 12, 2022 that may be closed by this pull request
Buffering policy argument in open() function call (python3 branch) #2158
Closed
@takashikumagai
Copy link
Contributor Author

I got to admit I'm not too familiar with this.. I'm trying to figure this one out
So what I did wrong is that all I did is on the python3 branch?

@MichaIng
Copy link
Member

Ah no I saw it wrong. The issue is that you did not rebase your branch, but that you merged dev via merge commit into your branch, which are now all shown as changes. However, I had a look over it and there is nothing wrong. Since all commits are there, it cannot be rebased anymore. It's fine, let's get it merged.

@takashikumagai
Copy link
Contributor Author

Ugh, I'm sorry about this.

MichaIng
MichaIng requested changes Mar 12, 2022
View reviewed changes
Copy link
Member

@MichaIng MichaIng left a comment

Choose a reason for hiding this comment

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

Ah wait. It may cause issues with other PRs now, especially since your original commit which changes a removed/moved file is still present.

I think easiest is to create a new PR or a new branch at least, based on dev with only single change done in a single commit.

@takashikumagai
Copy link
Contributor Author

That sounds like something I can do.

@takashikumagai
Copy link
Contributor Author

I'll create a new fork with the current dev branch, then make the same change and make a new pull request. Does that sound right?

@MichaIng
Copy link
Member

No problem btw. I think in GitHub web interface rebasing is actually not possible, a major lack. This would need to be done via git or gh command line utility or GitHub desktop. We all needed to learn this once, at least when dealing with PRs where conflicts have been merged into the base branch 🙂.

@MichaIng
Copy link
Member

I'll create a new fork with the current dev branch, then make the same change and make a new pull request. Does that sound right?

🚀

@takashikumagai
Copy link
Contributor Author

Well, I'm not familiar with the web interface, tbh. But if I re-create the same commit and make a very standard merge request, it will be okay I hope? I'll be doing so in a minute.

@MichaIng
Copy link
Member

Jep that should work fine.

@takashikumagai
Copy link
Contributor Author

Just created a very minimalistic PR for the dev branch. Can you take a look?

@takashikumagai
Copy link
Contributor Author

Thank you so much for your patience. Couldn't have done it without your help. 🐱

@MichaIng MichaIng mentioned this pull request Mar 12, 2022
Closed
This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@MichaIng MichaIng MichaIng requested changes

Assignees

@takashikumagai takashikumagai

Labels
python update python 2 > python 3
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Buffering policy argument in open() function call (python3 branch)
8 participants
@takashikumagai @zagrim @MichaIng @herostrat @jmichault @cclauss @mercutiodesign @doubleO8