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 tilde filename expansion to rfits/wfits #209
Conversation
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.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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? |
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.