[issue19696] Merge all (non-syntactic) import-related tests into test_importlib
aeros167 added the comment: Decided to start working on this issue, seems like a solid starting point. After submitting a minor doc contribution, I wanted to also attempt to contribute to some of the easier enhancement requests. >From my current assessment, it appears that "test_pkgimport", >"test_threaded_import", and "threaded_import_hangers" all would appropriately >be categorized in test_importlib. Pkgimport attempts to build the package, >perform basic filesystem tests, and ensure that run time errors are correctly >functioning. "threaded_import" attempts to simultaneously important multiple >modules by assigning each one to different threads. The purpose of >"threaded_import_hangers" is specifically dependent on "threaded_import", so >those two should be grouped together. While going through test_pkgimport I noticed that it uses the method "random.choose" on lines 17 and 63. However, the standard seems to "random.choice". I could not find any specific mention of random.choose on the python docs, so this appears to be a deprecated method. As far as I'm aware "random.choice" follows current standard and is used far more frequently within the CPython repository. Would it be appropriate to change the "choose" method to "choice"? Additionally, as far as naming convention goes, should the name of "test_pkgimport" instead be "test_pkg_import"? This is entirely semantics, but it makes for better consistency with the other file names and is a bit more readable. Also, would it be best to perform all of these changes within separate pull requests, or are they minor enough to be combined into one? -- nosy: +aeros167 ___ Python tracker <https://bugs.python.org/issue19696> ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue19696] Merge all (non-syntactic) import-related tests into test_importlib
aeros167 added the comment: > Yep, just make sure the tests still pass before and after the change. :) Sounds good, the first thing I had done before proposing the change was testing it in an IDE and using some logging to ensure that random.choose and random.choice were providing the same functionality. So hopefully the PR tests will pass as well. > Separate are easier to review. Alright, I'll do each of the file moves in individual PRs and then a separate one for changing random.choice to random.choose. Thanks! -- ___ Python tracker <https://bugs.python.org/issue19696> ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue19696] Merge all (non-syntactic) import-related tests into test_importlib
Change by aeros167 : -- keywords: +patch pull_requests: +14127 stage: needs patch -> patch review pull_request: https://github.com/python/cpython/pull/14303 ___ Python tracker <https://bugs.python.org/issue19696> ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue19696] Merge all (non-syntactic) import-related tests into test_importlib
aeros167 added the comment: Created a PR for moving and renaming "test_pkimport". An initial approval was made, now it is awaiting core review (https://github.com/python/cpython/pull/14303). -- ___ Python tracker <https://bugs.python.org/issue19696> ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue19696] Merge all (non-syntactic) import-related tests into test_importlib
aeros167 added the comment: > Thanks for the PR, aeros167! BTW, if you want to open a new issue and > modernize the tests to use importlib directly that would be great! Sounds good, I'll definitely look into doing that after finishing up this issue. Was waiting the previous PR to be merged before replacing the deprecated method "random.choose" with "random.choice" in "test_pkg_import.py" and then also moving the other two into test_importlib. Thank you very much for the timely feedback. This has been a great experience for learning to contribute to larger open source projects. It's been an aspiration of mine to give back to Python, as it was my first serious programming language 5 years ago. This may be an incredibly minor contribution in the grand scheme of things, but my goal is for it to be the first of many (: -- ___ Python tracker <https://bugs.python.org/issue19696> ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue19696] Merge all (non-syntactic) import-related tests into test_importlib
Change by aeros167 : -- pull_requests: +14283 pull_request: https://github.com/python/cpython/pull/14466 ___ Python tracker <https://bugs.python.org/issue19696> ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue19696] Merge all (non-syntactic) import-related tests into test_importlib
aeros167 added the comment: Created a new PR replacing the deprecated method "random.choose" with "random.choice" in "test_pkg_import.py" (https://github.com/python/cpython/pull/14466). -- ___ Python tracker <https://bugs.python.org/issue19696> ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue37462] Default starting directory for os.walk()
New submission from aeros167 : As a quality of life enhancement, it would beneficial to set a default starting directory for os.walk() to use when one is not specified. I would suggest for this to be the system's current working directory. The implementation would be rather straightforward, and should not cause any compatibility issues. In the function definition for walk in os.py, it would simply need to be changed from "def walk(top, topdown=True, onerror=None, followlinks=False)" to "def walk(top=".", topdown=True, onerror=None, followlinks=False)". As a separate topic but relevant to os.walk(), the current name of the positional argument which specifies the starting directory is currently called "top". However, a more descriptive word for the argument would probably be "root", "dir", "directory", or "start". The term "top" is quite generic and is not used to describe a filesystem position. Personally, I'm the most in favor of "root" as it describes the highest parent node of a tree, and provides an accurate description of its purpose. However, I will hold off on adding a PR for this change until some feedback is added, as it is not essential to the initial issue. Changing the name of argument could cause potential compatibility issues if the name is specified when using os.walk(), such as in "os.walk(top="dirpath")". However, doing so would be significantly unconventional. In the majority of common use cases, os.walk() only uses the first argument, so there is no need to specify the keyword. Even when more than one is used, usually the first argument is specified by position rather than keyword. -- components: Library (Lib) messages: 346961 nosy: aeros167 priority: normal severity: normal status: open title: Default starting directory for os.walk() type: enhancement versions: Python 3.9 ___ Python tracker <https://bugs.python.org/issue37462> ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue37462] Default starting directory for os.walk()
Change by aeros167 : -- keywords: +patch pull_requests: +14312 stage: -> patch review pull_request: https://github.com/python/cpython/pull/14497 ___ Python tracker <https://bugs.python.org/issue37462> ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue37462] Default starting directory for os.walk()
aeros167 added the comment: Created a new PR which sets the default value of top to the current working directory. (https://github.com/python/cpython/pull/14497) -- ___ Python tracker <https://bugs.python.org/issue37462> ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue37462] Default starting directory for os.walk()
aeros167 added the comment: Oh okay, that's fair enough. I can definitely understand that assigning the current directory as the default does not provide a significant change in improved functionality, and it is not implicit that os.walk() would use the current directory as the default option. Even if this change was approved, it is likely that many would never notice a difference. After reading the criticisms, I can also understand that the value of this feature would be too niche and does not justify the maintenance. It may also lead to some unnecessary confusion. Thanks for the constructive feedback serhiy and taleinat. I'm quite new to making contributions to Python (only a couple of minor merged PRs) and working on open source projects in general. This was my first attempt at coming up with an original issue for Python. As a result, I'm not entirely certain as to what qualifies as being adequately valuable enough to justify creating an issue. I have some basic understanding from others that I've looked over, but it seems like it might be a bit of a trial by error process. Before attempting to create any more original issues, I'll work on existing issues and the suggestions of others to develop a better practical understanding of the standards. I'll close this issue and the pull request, but I'll be sure to read over any other responses that are made. I definitely appreciate the criticism. -- stage: patch review -> resolved status: open -> closed ___ Python tracker <https://bugs.python.org/issue37462> ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue37462] Default starting directory for os.walk()
aeros167 added the comment: For any future issues, is it more appropriate to leave the issue status on pending until the proposal is approved? This may not apply if the issue was specifically mentioned or requested by a core developer, but it may be better to use pending for any original issues that I propose until they have received feedback. -- ___ Python tracker <https://bugs.python.org/issue37462> ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
