Migrate code from Python 2.7.x to Python 3 #51
Comments
|
@ibnesayeed I have not checked compatibility of all of the included libraries w/ Py3. What advantage would we gain in doing so? There are more fundamental problems w/ the project to be tackled than updating the code to the latest version of the language. |
|
At this point it is not a priority, but for the longevity of the project it should not use the version of the language that is nearing the extended EOL date. |
|
Ok, it will be considered in the future when more specific rationales are put forth. |
ibnesayeed
modified the milestones:
2.0 (Extended more featureful implementation),
1.X (Refined, less buggy implementation)
|
|
Same commands from virtualenv: $ ipwb
Traceback (most recent call last):
File "/usr/local/bin/ipwb", line 11, in <module>
load_entry_point('ipwb==0.2016.12.10.1711', 'console_scripts', 'ipwb')()
File "/usr/local/lib/python2.7/site-packages/pkg_resources/__init__.py", line 561, in load_entry_point
return get_distribution(dist).load_entry_point(group, name)
File "/usr/local/lib/python2.7/site-packages/pkg_resources/__init__.py", line 2627, in load_entry_point
return ep.load()
File "/usr/local/lib/python2.7/site-packages/pkg_resources/__init__.py", line 2287, in load
return self.resolve()
File "/usr/local/lib/python2.7/site-packages/pkg_resources/__init__.py", line 2293, in resolve
module = __import__(self.module_name, fromlist=['__name__'], level=0)
File "/usr/local/lib/python2.7/site-packages/ipwb/__main__.py", line 7, in <module>
from . import indexer
File "/usr/local/lib/python2.7/site-packages/ipwb/indexer.py", line 1, in <module>
Non
NameError: name 'Non' is not defined |
|
It looks like |
machawk1
changed the title
|
@ibnesayeed The standard approach appears to be to make the code compatible with both Py2 and Py3. Would that be a suitable goal for this ticket or is it important that it ONLY be compatible with Py3? |
|
Supporting both Py2 and Py3 is a good idea for tools that have wider user base of which some might be using legacy systems or cant upgrade their Python environment. It is also a good idea for libraries/packages that might be used in different programs. Other than that I don't see a reason to support both which will increase our test burden. Additionally, it would encourage the use of a version of Python that is reaching it's extended end of life (which won't be a good service for the Python community that is struggling to get rid of the older version). People often use a compatibility library called six to facilitate this, but I don't really see a need of that in this repo. |
|
@ibnesayeed So what's your vote? It still seems that a large majority of users have Py2 by default, so might be discouraged when installing ipwb and seeing that it immediately fails if we only cater to Py3. I'm all for using six and am admittedly way more familiar with Py2 myself. Despite the overhead in testing, we currently have very minimal test cases written, so adding the extra test cases to account for both Py variants might make the code more resilient. Regarding this last point, we can also try to make the test code as Py version agnostic as possible to mitigate the potential redundancy of supporting both versions. |
|
When installing in Python3 after fixing relative imports, running Update: Further update: |
|
On the other hand, I would go for Py3. Our tool is perhaps not going to be used by newbies. We expect our users to be advance who are familiar with the bleeding edge technology like IPFS. When I said testing overhead, it was not just about test cases, but also performing tests on various environments. |
Wrong. We want to make the tool more accessible and useful to anyone with WARCs or who are passed an index file (for now) and simply want to replay the collection. I think macOS still ships only w/ Py2 by default. |
|
Well, I still think it is an exploratory effort with more stable and practical model to be developed later (#61). If you want to make it accessible to more people then perhaps the right approach would be to code with Py3 as the target platform, but provide backwards compatibility using six. This way it will be easier to get rid of Py2 support in future. |
|
@ibnesayeed That seems like the right approach. I'll continue to document efforts in this ticket. |
|
Preliminary work is being done in https://github.com/oduwsdl/ipwb/tree/issue-51 . One weird change was |
|
Yet another reason, https://github.com/ipfs/py-ipfs-api says py2 is deprecated. |
|
I am of the opinion to fully migrate it to Py3 and completely discontinue support for Py2 (not even opportunistic support). As Py3 is becoming more ubiquitous, we will see more people complaining about the software failing in their environment (of which some might not even bother to report and just move on). An attempt to support both versions is counter-productive that leaves larger failure and support surface. |
|
Thanks for your 2¢ @ibnesayeed . 😃 A few reasons for my original concerns and rationale for retaining Py2 support was:
That said, I agree that migration is or will be needed. The benefits at the moment, however, do not outweigh the work required to do so. I fear there's more to it than running Py2to3. |
|
PyInstaller's documentation does not seem to suggest that their Py3 support is any less stable than Py2. If you really think that there are substantial number of IPWB users who might have Py2 and not Py3, then it is OK to continue supporting that. However, Py3 support should be prioritized (which, I think, is broken at the moment). |
|
I feel it will be best to first make ipwb cross-compatible then we can evaluate the overhead in maintaining the Py2 code bits to determine whether it will worthwhile to retain support for both versions. |
|
Looks like a good approach to give it a try. |
|
Let's prioritize this. In the DWeb Summit some people gave me that "Still Py2?" look! |
|
@ibnesayeed We have quite a dependency chain but I think giving this priority is correct. One such soft dependency before the transition is to have more rigorous testing to ensure consistency of functionality for the plethora of edge cases that are not currently automatically tested. Having those in-place would be good for some quasi regression testing but I believe it will just cause us to drag our feet more in moving to Py3. The various futzing and pattern of closing tickets we created the same day has also been a barrier in making progress on the big lingering ones (like #51). |
|
An updated TODO (check-)list of the remaining parts of the code that need to be converted to Py3 would be useful here and facilitation completion of this ticket. |
|
As verbally discussed, one of the barriers in completing this is adapting the test "suite" to work in Py3. The code itself seems to work as expected. When running the tests using the docker build process after changing the Dockerfile to use Python 3.7, the main complaint is the tests' inability to find our |
|
Perhaps from within the tests, instead of using EDIT: This causes pytest to complain about other things (namely, urllib2), so perhaps progress: |
|
Finally closed in #609. 🙌 |
@machawk1, is there anything that is stopping us from moving this to to Python 3.x?
The text was updated successfully, but these errors were encountered: