Todd Rovito added the comment:
Retested this patch with the latest 3.4 and made one tiny change to get it to
apply cleanly. Please provide feedback or commit. I would like to get this
committed after more than a year since the original bug report.
--
Added file:
Todd Rovito added the comment:
I forgot to mention this patch only works on 3.4 but if it is committed I will
work on a patch for 2.7. Thanks.
--
___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue16278
Todd Rovito added the comment:
Ping!!!
I have not heard anything about this patch so I wanted to ping it to get more
feedback. Thanks!
--
___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue16278
Todd Rovito added the comment:
V3 added which includes much shorter variable names and a cleanup of the patch.
I still need to test on Windows.
--
Added file: http://bugs.python.org/file29370/16278OSRenameDocsTestV3.patch
___
Python tracker
Todd Rovito added the comment:
Thanks for the feedback Vitaliy Stepanov that helped alot with the version 3
patch.
--
___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue16278
___
Todd Rovito added the comment:
Made some very minor changes to get patch to work on Windows. Had issues with
line ending differences between Unix and Windows. I fixed it so line endings
won't be an issue and tested patch on Windows, Linux, and OS X.
Please feel free to supply feedback or
Todd Rovito added the comment:
Version 2 of the patch includes many improvements most of which were suggested
by Ezio making the patch a much higher quality. The patch is very WET at this
point. I still need to test on Windows which I plan to do as soon as I can get
my Windows partition in
Ezio Melotti added the comment:
Adding more tests is good, even though there are still a few things that should
be improved (see comments on rietveld).
Regarding the documentation I'm not sure it's a good idea to be so detailed.
On one hand, if we test the behavior we can make sure that the
Todd Rovito added the comment:
Ezio,
Thank you or the feedback I will continue to polish the test cases. As far
as the documentation I really like the table and find it easy to read but I
could go either way. If you read the history of this issue other Python Core
Developers asked to
Changes by Todd Rovito rovit...@gmail.com:
Removed file: http://bugs.python.org/file27890/OSRename_test_os_3point4.patch
___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue16278
___
Changes by Todd Rovito rovit...@gmail.com:
Removed file: http://bugs.python.org/file28071/OSRenameDocs3point4.patch
___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue16278
___
Todd Rovito added the comment:
Combined the test cases and document changes into a single patch. As suggested
by Mr. Terry Reedy I used a table and removed the text. The table still needs
some work but it is a good start. As suggested by Mr. Jerdonek I tried to make
the test cases WETter
Todd Rovito added the comment:
Over the weekend I caught this terrible cold and have not been able to work on
this issue much. Hopefully I can complete by next weekend 2/25/2013 thanks for
your understanding.
--
___
Python tracker
Todd Rovito added the comment:
Thanks Terry and Chris you guys have supplied great feedback. I will work on
the issue and try to get a new patch updated by end of the weekend (2/18/13).
Sent from my iPhone
On Feb 13, 2013, at 11:56 PM, Terry J. Reedy rep...@bugs.python.org wrote:
Terry
Senthil Kumaran added the comment:
The doc change looks good to me. I am adding Terry and Ezio to see if they have
any comments on wordings in the doc. If not, I can commit this change. I think
that this is helpful.
--
nosy: +ezio.melotti, orsenthil, terry.reedy
Chris Jerdonek added the comment:
I commented above that the tests are not very DRY though. Shouldn't there be
tests to check that the documented behavior is correct?
--
___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue16278
Senthil Kumaran added the comment:
Chris: The patch is for the docs. the test code I believe, is for
reference. It would be to run it on different OS and verify the
documentation matches the behavior.I am okay with more than one person
verifying this and I shall do on OS'es I have.
--
Todd Rovito added the comment:
Chris,
I first verified the issue then created some wording and you pointed out
that we needed test cases so I created a bunch of test cases. As you pointed
out I count 2*3*2=12 possibilities to check (excluding src and dst being on
different filesystems):
Chris Jerdonek added the comment:
Senthil, in my experience, whenever documentation is added that documents new
aspects of behavior (e.g. is not just a rewording of existing documentation),
tests are always added simultaneously (if not already present) to ensure that
the code doesn't regress
Chris Jerdonek added the comment:
If it would help I could submit a single patch.
Yes, a single patch is best. One way to make code more DRY is refactoring to
use helper functions as needed (i.e. same code in multiple places - one helper
function).
--
Todd Rovito added the comment:
Chris,
Thanks for the clarification. I thought you were telling me my test cases
were dry as in dry humorI will read-up on the dry concept and see what I
can do to consolidate plus I will combine the patches. Initially I am thinking
I could collapse all
Chris Jerdonek added the comment:
The number of test cases isn't the problem. Having more but finer-grained test
cases is usually preferred in fact. It's just that the test cases should be
sharing code where possible so that they're shorter and easy to see how they
differ from one another.
Terry J. Reedy added the comment:
directory name.Yet a needs spaces after '.'.
The text is decent English and clear enough sentence by sentence, but the
reality and hence the text as a whole is confusing.
I wonder if it would be possible to make a little table
dest
Todd Rovito added the comment:
This is a gentle ping of this issue. Can somebody please review and let me
know what needs to be done to get this committed? I did test the patch on
2/12/2013 and it seems to work from the latest 3.4. Thanks!
--
Todd Rovito added the comment:
Attached is patch with the final formatting for the documentation updates. I
fixed the :exc:`OSError` problems that I had before and used indents to denote
Unix behavior VS Windows behavior. Please let me know if I can do anything else
to help get this issue
Changes by Todd Rovito rovit...@gmail.com:
Removed file: http://bugs.python.org/file27640/OSRenameTest3point4.patch
___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue16278
___
Todd Rovito added the comment:
Here is a draft suggestion for the documentation change, not all the formatting
is worked out:
.. function:: rename(src, dst, *, src_dir_fd=None, dst_dir_fd=None)
Rename the file or directory *src* to *dst*. If *src* exists as either
a file or directory
Todd Rovito added the comment:
Chris,
That is an excellent suggestion. I modified my OSRenameCombinations.py
program and attached. This program prints a table with the src parameters as
columns and the dst as rows. Hopefully it will show up ok in the bug tracker.
For Unix
Todd Rovito added the comment:
Attached is a patch for 16 test cases. All 16 test cases have been tested on
Windows 7, Mac OS X, and Linux they seem to function well. Before this patch
there was only a single test case for rename. For each test case I used
unittest.skipUnless to make sure
Chris Jerdonek added the comment:
Attached is a patch for 16 test cases.
The test cases look quite verbose (e.g. they're not DRY), but it's a good
start. Thanks. For others' benefit, can you perhaps summarize your findings
concisely in a table/chart of some form?
--
Todd Rovito added the comment:
While writing test cases I discovered another conflict with the documentation.
The phrase On Unix, if dst exists and is a file, it will be replaced silently
if the user has permission and src is a file. is not correct. According to
the test cases I wrote in
Changes by Todd Rovito rovit...@gmail.com:
Removed file: http://bugs.python.org/file27804/OSRename_test_os.patch
___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue16278
___
Todd Rovito added the comment:
False alarm my test case was buggy.
--
___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue16278
___
___
Python-bugs-list
Todd Rovito added the comment:
Over the weekend I verified the test cases are incomplete for Python 3.4.
Inside of Lib/test/test_os.py is a class FileTests that contains a single
function test_rename which seems to only check to make sure that the reference
count for the first argument is
Todd Rovito added the comment:
Thanks for the feedback! Over the weekend I will make sure the documentation
and test cases cover all possibilities. I have not worked with test suite but
I will do my best.
--
___
Python tracker
Changes by Andrew Svetlov andrew.svet...@gmail.com:
--
nosy: +asvetlov
___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue16278
___
___
Andrew Svetlov added the comment:
Chris, maybe you can guess better wording?
--
nosy: +chris.jerdonek
___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue16278
___
Chris Jerdonek added the comment:
Before we discuss wording, it's not clear to me whether all possibilities have
been checked. I count 2*3*2=12 possibilities to check (excluding src and dst
being on different filesystems):
src: file or directory
dst: file, empty directory, or non-empty
Changes by Todd Rovito rovit...@gmail.com:
--
nosy: +Todd.Rovito
___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue16278
___
___
Python-bugs-list
Todd Rovito added the comment:
David,
Thanks for your bug report. Indeed os.rename does not exhibit the same
behavior as the documentation describes.
For Python 3.4 here is the fix I came up with:
Rename the file or directory src to dst. If dst is a directory that is not
empty, OSError
New submission from David Benjamin:
This is somewhat of a nitpick. os.rename's documentation says If dst is a
directory, OSError will be raised. On Unix, this isn't completely true. If the
source is a directory and the destination is an empty directory, it'll
overwrite the former with the
Changes by Serhiy Storchaka storch...@gmail.com:
--
versions: +Python 3.2, Python 3.3 -Python 3.5
___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue16278
___
42 matches
Mail list logo
