bin/pysmu: Fix urllib imports for python2 and python3 compatibility

cyber-g commented 3 years ago

1/ Fix "ImportError: cannot import name 'urlretrieve'" 2/ Fix "ModuleNotFoundError: No module named 'urllib2'"

Fixes #174

AppVeyorBot commented 3 years ago

:x: Build libsmu 1.0.1341 failed (commit https://github.com/analogdevicesinc/libsmu/commit/6af3cd8d9f by @cyber-g)

AppVeyorBot commented 3 years ago

:x: Build libsmu 1.0.1342 failed (commit https://github.com/analogdevicesinc/libsmu/commit/71b219fa5d by @cyber-g)

cristi-iacob commented 3 years ago

1) For solving this issue we have to consider the situation for both Python2 and Python3. So a solution for this issue might be the one I described in #174. Can you please modify your commit to contain the solution which is both Python2 and Python3 friendly? (The current one is only Python3 friendly). If you want I can do this for you. Just let me know. 1.1) More like a general thing about libsmu commits. In order to be accepted into the master branch, your commits should respect a pattern. The main things to respect are:

the title should have the following pattern: <file/module name>: </li> <li>the commit should contain a brief description of what you have done in it.</li> <li>the commit must be signed. </li> </ul> <p>You can check one of the latest commits from the master branch in order to see an example (ex. 88aa883e4324c6477fb84c29213578920e27de78)</p> <p>2) Thank you for your interest in improving the pysmu examples. But please keep a pull request for each feature you want to merge and don't mix them together. Just to be more clear, in this case you should have opened two pull requests - one for the pysmu fix and one for the new example. </p> <p>Although your script is a little more concise then the existing one, I think that it would be better to keep the old one as it is a little more specific, even with the few extra commands. The reasoning is that these examples have the main role of making the people better understand how pysmu works and not necessarily to be the shortest solution for that matter. For example, the for loop that iterates through the samples shows what structure a sample has. </p> <p>Just to sum up, can you please modify this pull request to only contain the pysmu CLI related fixes and do the specified modifications? Let me know if you need any help with this and thank you again for your interest! :)</p> </div> </div> <div class="comment"> <div class="user"> <a rel="noreferrer nofollow" target="_blank" href="https://github.com/cyber-g"><img src="https://avatars.githubusercontent.com/u/19309536?v=4" />cyber-g</a> commented <strong> 3 years ago</strong> </div> <div class="markdown-body"> <p>Thanks for your reply. </p> <ol> <li>"...Can you please modify your commit to contain the solution..." : Sure, I will work on that 1.1) "The main things to respect are" : Thanks for the instructions, I did not know the rules. I will work on the compliance. </li> <li>"...the pysmu examples..." : I am so sorry about that ; it is actually an unwanted action. I am currently learning to use git. My example has been merged to the pull request but I actually made its commit after the pull request... I guess I should have created a specific branch for that. I will remove the example from the pull request. </li> </ol> </div> </div> <div class="comment"> <div class="user"> <a rel="noreferrer nofollow" target="_blank" href="https://github.com/AppVeyorBot"><img src="https://avatars.githubusercontent.com/u/17834419?v=4" />AppVeyorBot</a> commented <strong> 3 years ago</strong> </div> <div class="markdown-body"> <p>:x: <a href="https://ci.appveyor.com/project/analogdevicesinc/libsmu/builds/37291426">Build libsmu 1.0.1343 failed</a> (commit <a href="https://github.com/analogdevicesinc/libsmu/commit/a52b1c05ce">https://github.com/analogdevicesinc/libsmu/commit/a52b1c05ce</a> by @cyber-g)</p> </div> </div> <div class="comment"> <div class="user"> <a rel="noreferrer nofollow" target="_blank" href="https://github.com/cyber-g"><img src="https://avatars.githubusercontent.com/u/19309536?v=4" />cyber-g</a> commented <strong> 3 years ago</strong> </div> <div class="markdown-body"> <p>I have activated GPG signature on my computer as described in: <a href="https://docs.github.com/en/free-pro-team@latest/github/authenticating-to-github/signing-commits">https://docs.github.com/en/free-pro-team@latest/github/authenticating-to-github/signing-commits</a> And I had to enter some passphrase during my commit. </p> <p>I was expecting the <code>Signed-off-by:...</code> label at the end of my commit message to be automatically added by git but I understand that it is not the case... </p> </div> </div> <div class="comment"> <div class="user"> <a rel="noreferrer nofollow" target="_blank" href="https://github.com/cyber-g"><img src="https://avatars.githubusercontent.com/u/19309536?v=4" />cyber-g</a> commented <strong> 3 years ago</strong> </div> <div class="markdown-body"> <p>I can see that the automatic build failed but I lack experience in CI. I read the build log and see some deprecation warnings in the middle and at the end: </p> <pre><code>:: Proceed with installation? [Y/n] :: Retrieving packages... error: failed retrieving file 'zstd-1.4.1-1-x86_64.pkg.tar.xz' from repo.msys2.org : The requested URL returned error: 404 error: failed retrieving file 'zstd-1.4.1-1-x86_64.pkg.tar.xz' from sourceforge.net : The requested URL returned error: 404 error: failed retrieving file 'zstd-1.4.1-1-x86_64.pkg.tar.xz' from www2.futureware.at : The requested URL returned error: 404 error: failed retrieving file 'zstd-1.4.1-1-x86_64.pkg.tar.xz' from mirror.yandex.ru : The requested URL returned error: 404 warning: failed to retrieve some files error: failed to commit transaction (unexpected error) Errors occurred, no packages were upgraded. Command exited with code 1</code></pre> <p>Is this error actually related to my code modification ?</p> </div> </div> <div class="comment"> <div class="user"> <a rel="noreferrer nofollow" target="_blank" href="https://github.com/cristi-iacob"><img src="https://avatars.githubusercontent.com/u/52404495?v=4" />cristi-iacob</a> commented <strong> 3 years ago</strong> </div> <div class="markdown-body"> <p>About the sign-off: git does not automatically add the sign-off label. You should add it manually. You can find more details about it <a href="https://docs.pi-hole.net/guides/github/how-to-signoff/">here</a>.</p> <p>About the CI: no, that issue is not related to your code. I will fix it. </p> <p>About the pull request: can you please restructure your commits? Your pull request should only contain one commit, containing only the modifications from the pysmu file. Just to be more clear, right now the pull request contains 3 commits: </p> <ol> <li>the one in which you made the initial modifications (which you modified afterwards in the third commit);</li> <li>the one in which you implemented the example;</li> <li>the one in which you modified the pysmu file with the complete solution and deleted the example.</li> </ol> <p>The pull request should contain only one commit, containing only the final solution for the pysmu file (the one from the third commit). In order to do this, you should delete the first two commits and modify the last one (to remove the example file deletion, as it won't exist anymore in the git history after you delete the second commit). Even if the result is the same, it is better to keep the git history as clean as possible and to remove the redundant modifications. Please let me know if you need any help for doing these modifications! Thank you! </p> </div> </div> <div class="comment"> <div class="user"> <a rel="noreferrer nofollow" target="_blank" href="https://github.com/cyber-g"><img src="https://avatars.githubusercontent.com/u/19309536?v=4" />cyber-g</a> commented <strong> 3 years ago</strong> </div> <div class="markdown-body"> <p>Thank you for your feedback. Would you have any example/tutorial on redoing the PR ? I am unsure of the right operation to perform with git now. (still learning the git workflow) I just need the right command to start again ; is it right if I use : <code>git reset --soft 4d04dcd7615588d8f65d4d1634024435c231ec47</code> and redo the clean commit ?</p> <p>(I have put <code>4d04dcd7615588d8f65d4d1634024435c231ec47</code>, which is the ID of last commit to the project.)</p> </div> </div> <div class="comment"> <div class="user"> <a rel="noreferrer nofollow" target="_blank" href="https://github.com/cristi-iacob"><img src="https://avatars.githubusercontent.com/u/52404495?v=4" />cristi-iacob</a> commented <strong> 3 years ago</strong> </div> <div class="markdown-body"> <p>Yes, the <code>git reset --soft <hash_id></code> should solve the issue. After you modify your commit structure, you should force push the modified branch to the branch from your personal repository (cyber-g:master). Note that you should use the --force option for the git push instruction as your local branch has degenerated from the remote one. </p> <p>As an alternative, you might consider using the <a href="https://git-scm.com/book/en/v2/Git-Tools-Rewriting-History">git interactive rebase</a> instead of the git reset command. This is a little more complicated method, but it is quite a powerful git tool. In this case, you should use <code>git rebase -i HEAD~3</code> (to rebase the last 3 commits). You should edit the text from the editor which pops up, dropping the first 2 commits and editing the last one. Now you can amend the last commit (removing the example deletion from it). </p> <p>Before pushing the local branch to the remote, please make sure that the branch history is correct. The last 2 commits should be your new one and the last one from the analogdevicesinc:master (make sure that it keeps its hash, 4d04dcd76). You can verify this by using <code>git log</code>. And don't forget that you should use <code>git pull --force</code> because you modified the history. After you push your branch, the pull request will be automatically modified. Please let me know if there is anything else I can help you with. Thank you! </p> </div> </div> <div class="comment"> <div class="user"> <a rel="noreferrer nofollow" target="_blank" href="https://github.com/AppVeyorBot"><img src="https://avatars.githubusercontent.com/u/17834419?v=4" />AppVeyorBot</a> commented <strong> 3 years ago</strong> </div> <div class="markdown-body"> <p>:x: <a href="https://ci.appveyor.com/project/analogdevicesinc/libsmu/builds/37343462">Build libsmu 1.0.1344 failed</a> (commit <a href="https://github.com/analogdevicesinc/libsmu/commit/fac8e98fbd">https://github.com/analogdevicesinc/libsmu/commit/fac8e98fbd</a> by @cyber-g)</p> </div> </div> <div class="comment"> <div class="user"> <a rel="noreferrer nofollow" target="_blank" href="https://github.com/cyber-g"><img src="https://avatars.githubusercontent.com/u/19309536?v=4" />cyber-g</a> commented <strong> 3 years ago</strong> </div> <div class="markdown-body"> <p>Thanks for your advices : I finally used <code>git rebase</code> which brought me to discover more elements on git workflows. </p> <p>I had to write an additional commit message when I performed the <code>git pull --force</code> ; I improvised on this last one. Let me know if I have to refine it. </p> </div> </div> <div class="comment"> <div class="user"> <a rel="noreferrer nofollow" target="_blank" href="https://github.com/cristi-iacob"><img src="https://avatars.githubusercontent.com/u/52404495?v=4" />cristi-iacob</a> commented <strong> 3 years ago</strong> </div> <div class="markdown-body"> <p>Sorry, I meant <code>git push --force</code> instead of <code>git pull --force</code>. Sorry for this. The pull command brings all the commits from the remote branch to the local one and basically merge them, that's why it requested you to make a merge commit. Anyway, that's not the case, because you are not supposed to run <code>git pull --force</code> (at least not in this case, as the remote master branch didn't change).</p> <p>Please do another rebase, deleting all the commits from this branch besides the fourth one (by dropping them - choose the <code>d</code> option for them in the interactive rebase file). At the end your branch's last 2 commits should be the current fourth one and the last commit from the remote master branch (4d04dcd). Afterwards you should run <code>git push --force</code>. You can check if you done this successfully by checking the commits tab from the top of the pull request. Please let me know if there is anything else I can help you with! Thank you! </p> </div> </div> <div class="comment"> <div class="user"> <a rel="noreferrer nofollow" target="_blank" href="https://github.com/cyber-g"><img src="https://avatars.githubusercontent.com/u/19309536?v=4" />cyber-g</a> commented <strong> 3 years ago</strong> </div> <div class="markdown-body"> <p>I reused the <code>git rebase</code> to modify the commit structure. Let me know if it should refined again. </p> <p>I learnt a lot during this process and I say it again : Many thanks for your help. </p> </div> </div> <div class="comment"> <div class="user"> <a rel="noreferrer nofollow" target="_blank" href="https://github.com/AppVeyorBot"><img src="https://avatars.githubusercontent.com/u/17834419?v=4" />AppVeyorBot</a> commented <strong> 3 years ago</strong> </div> <div class="markdown-body"> <p>:x: <a href="https://ci.appveyor.com/project/analogdevicesinc/libsmu/builds/37345038">Build libsmu 1.0.1345 failed</a> (commit <a href="https://github.com/analogdevicesinc/libsmu/commit/37009b9800">https://github.com/analogdevicesinc/libsmu/commit/37009b9800</a> by @cyber-g)</p> </div> </div> <div class="comment"> <div class="user"> <a rel="noreferrer nofollow" target="_blank" href="https://github.com/cristi-iacob"><img src="https://avatars.githubusercontent.com/u/52404495?v=4" />cristi-iacob</a> commented <strong> 3 years ago</strong> </div> <div class="markdown-body"> <p>I'm glad to hear that this was an educative process for you! Thank you for your contribution! </p> <p>If you find any other issues with the libsmu library or you have ideas for new features, please feel free to present them to us! Thank you again! </p> </div> </div> <div class="page-bar-simple"> </div> <div class="footer"> <ul class="body"> <li>© <script> document.write(new Date().getFullYear()) </script> Githubissues.</li> <li>Githubissues is a development platform for aggregating issues.</li> </ul> </div> <script src="https://cdn.jsdelivr.net/npm/jquery@3.5.1/dist/jquery.min.js"></script> <script src="/githubissues/assets/js.js"></script> <script src="/githubissues/assets/markdown.js"></script> <script src="https://cdn.jsdelivr.net/gh/highlightjs/cdn-release@11.4.0/build/highlight.min.js"></script> <script src="https://cdn.jsdelivr.net/gh/highlightjs/cdn-release@11.4.0/build/languages/go.min.js"></script> <script> hljs.highlightAll(); </script> </body> </html>

analogdevicesinc / libsmu

bin/pysmu: Fix urllib imports for python2 and python3 compatibility #175