Add tilde filename expansion to rfits/wfits #209
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
We use the same tilde expansion that is used in PDL::AutoLoader, so there should be no (additional) portability issues. It would probably be better to put this into a new IO.pm module and then all the PDL::IO::* modules could use it.
Previously the first element of a list provided to 'cat' was used to set the type of the output piddle. Now it uses the "highest" type if passed a mixed-type list. Also added some tests for this new behavior, and improved the documentation for cat, append, and glue to better highlight the differences between then.
Instead of the pwnam etc which is not super portable, we can use glob(~) to get the home directory. That should be more portable. Alternatively we could add a dependency and use File::HomeDir.
mohawk2
reviewed
Mar 4, 2018
t/fits.t
Outdated
| # Check that tilde expansion works | ||
| my $tildefile = cfile('~',"PDL-IO-FITS-test_$$.fits"); | ||
| eval { sequence(3,5,2)->wfits($tildefile); }; | ||
| ok(!$@, "wfits tilde expansion didn't fail"); |
There was a problem hiding this comment.
Since we're using Test::More these days, you could do this:
is $@, '', "wfits tilde expansion didn't fail";which has the added benefit of immediately telling you what $@ was if there was an error!
There was a problem hiding this comment.
We're also using Test::Exception, so that could even be
use Test::Exception;
lives_ok {
sequence(3,5,2)->wfits($tildefile)
} "wfits tilde expansion didn't fail";
|
This looks Ok to me. Unless anyone can see a reason not to, why not merge? |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
We use the same tilde expansion that is used in PDL::AutoLoader,
so there should be no (additional) portability issues.
It would probably be better to put this into a new IO.pm module
and then all the PDL::IO::* modules could use it.