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.

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

Add Path.absPath, file.absPath and associated tests #12394

Merged
merged 6 commits into from
Feb 27, 2019

Conversation

dlongnecke-cray
Copy link
Contributor

@dlongnecke-cray dlongnecke-cray commented Feb 27, 2019

Implement a working version of Path.absPath() and file.absPath() (both in the Path module). The Python 3.7.2 POSIX implementation of os.path.abspath() was referenced to ensure compatibility.

Python's file.name and Chapel's file.path seem to have different semantics (the former is the path used to open, the latter appears to be relative).

I'd appreciate some additional tests if you can think of them, particularly for file.absPath().

Test coverage:

  • darwin
  • cygwin64

Thanks @lydia-duncan for the review!

Resolves #6005.

Copy link
Member

@lydia-duncan lydia-duncan left a comment

Choose a reason for hiding this comment

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

I think the test cases you have are reasonable. The only thing that I can think of right now is a path involving a directory you don't have permissions on (like "foo/hiddenPath/../bar.txt" or something), and only because I was trying to think of weird cases. That'll be tricky to write - you'll want to take a look at the chmod tests for an example of how to make it and clean it up.

.gitignore Outdated Show resolved Hide resolved
modules/standard/Path.chpl Outdated Show resolved Hide resolved
modules/standard/Path.chpl Show resolved Hide resolved
modules/standard/Path.chpl Outdated Show resolved Hide resolved
modules/standard/Path.chpl Outdated Show resolved Hide resolved
test/library/standard/Path/dlongnecke/absPath/absPath.chpl Outdated Show resolved Hide resolved
Copy link
Member

@lydia-duncan lydia-duncan left a comment

Choose a reason for hiding this comment

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

These changes look good! I'm going to run a version myself and will give you the all clear when that passes

@lydia-duncan
Copy link
Member

One thing: it'd be good to include the testing you did in the PR/merge message

@dlongnecke-cray dlongnecke-cray merged commit c85c4dd into chapel-lang:master Feb 27, 2019
@dlongnecke-cray dlongnecke-cray deleted the path-abspath branch February 27, 2019 19:22
ronawho added a commit to ronawho/chapel that referenced this pull request Mar 15, 2019
Add recent annotations and backfill missing ones since last release

Improvements:
 - minor unordered operation improvements (chapel-lang#12089)
 - massive single and multi-locale scan improvements (chapel-lang#12469, chapel-lang#12481)
 - string-ish improvements from --no-checks disabling range checks (chapel-lang#11780)
 - remote-record-read comm count regression and fix (chapel-lang#11629, chapel-lang#12439)
 - tuple-to-complex cast regression fix (chapel-lang#12429)
 - sparse bulk-add improvement from using radix sort (chapel-lang#12452)
 - memory leak regressions, fixes, and improvements (chapel-lang#12225, chapel-lang#12394, chapel-lang#12421,
   chapel-lang#12512, chapel-lang#12500, chapel-lang#12516, chapel-lang#12527, chapel-lang#12531, chapel-lang#12551, chapel-lang#12554, chapel-lang#12552)

 - minor regression for serial reductions from ExternArr (chapel-lang#11893)

 - mark notable reboots of chapcs machines
ronawho added a commit that referenced this pull request Mar 15, 2019
Add perf annotations for 2019-03-14

Add recent annotations and backfill missing ones since last release

Improvements:
 - minor improvements for unordered operations (#12089)
 - massive single and multi-locale scan improvements (#12469, #12481)
 - string-ish improvements from --no-checks disabling range checks (#11780)
 - regression and fix for remote-record-read-copy comm count (#11629, #12439)
 - regression fix for tuple-to-complex cast (#12429)
 - sparse bulk-add improvement from using radix sort (#12452)
 - regressions, fixes, and improvements for leaks (#12225, #12394, #12421,
    #12512, #12500, #12516, #12527, #12531, #12551, #12554, #12552)

Regressions:
 - minor regression for serial reductions from ExternArr (#11893)

Misc:
 - mark notable reboots of chapcs machines (e.g. llvm, default)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants