diff --git a/CHANGELOG.md b/CHANGELOG.md index 0ce97256d..4aa19a0fb 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,12 @@ The original MRs are only visible on the [LIGO GitLab repository](https://git.li ## [Unreleased] +## [2.5.2] + +### Fixed + +- Fix the filename extension precedence for result files (https://github.com/bilby-dev/bilby/pull/960) + ## [2.5.1] ### Changed @@ -1151,7 +1157,8 @@ First `pip` installable version https://pypi.org/project/BILBY/ . - All chainconsumer dependency as this was causing issues. -[Unreleased]: https://github.com/bilby-dev/bilby/compare/v2.5.1...main +[Unreleased]: https://github.com/bilby-dev/bilby/compare/v2.5.2...main +[2.5.2]: https://github.com/bilby-dev/bilby/compare/v2.5.1...v2.5.2 [2.5.1]: https://github.com/bilby-dev/bilby/compare/v2.5.0...v2.5.1 [2.5.0]: https://github.com/bilby-dev/bilby/compare/v2.4.0...v2.5.0 [2.4.0]: https://github.com/bilby-dev/bilby/compare/v2.3.0...v2.4.0 diff --git a/bilby/core/result.py b/bilby/core/result.py index f33725c32..0e171df51 100644 --- a/bilby/core/result.py +++ b/bilby/core/result.py @@ -768,9 +768,8 @@ def _get_save_data_dictionary(self): return dictionary def save_to_file(self, filename=None, overwrite=False, outdir=None, - extension='json', gzip=False): + extension=None, gzip=False): """ - Writes the Result to a file. Supported formats are: `json`, `hdf5`, `pickle` @@ -785,22 +784,35 @@ def save_to_file(self, filename=None, overwrite=False, outdir=None, outdir: str, optional Path to the outdir. Default is the one stored in the result object. If given, overwrite path prefix in 'filename'. - extension: str, optional {json, hdf5, pkl, pickle, True} - Determines the method to use to store the data (if True defaults - to json) + extension: {"json", "hdf5", "pkl", None}, optional + Determines the method to use to store the data. If None, the extension + is inferred from the filename if provided, otherwise defaults to 'json'. gzip: bool, optional If true, and outputting to a json file, this will gzip the resulting file and add '.gz' to the file extension. """ - if extension is True: - extension = "json" - _outdir = None if filename is not None: - _outdir, filename = os.path.split(filename) + _outdir, base_filename = os.path.split(filename) _outdir = None if _outdir == "" else _outdir - filename = f"{os.path.splitext(filename)[0]}.{extension}" + base, ext = os.path.splitext(base_filename) + if extension is None: + extension = ext[1:] if ext else "json" + filename = base_filename + elif extension is True: + message = "Result.save_to_file called with extension=True. " + if len(ext) > 0: + message += f"Overwriting extension to json from {ext}, this" + else: + message += "This" + message += " behaviour is deprecated and will be removed." + logger.warning(message) + extension = "json" + filename = base_filename + filename = f"{base}.{extension}" + if extension is None or extension is True: + extension = 'json' outdir = _outdir if outdir is None else outdir outdir = self._safe_outdir_creation(outdir, self.save_to_file) @@ -842,13 +854,13 @@ def save_to_file(self, filename=None, overwrite=False, outdir=None, elif extension == 'pkl': safe_file_dump(self, output_path, "dill") else: - raise ValueError("Extension type {} not understood".format(extension)) + raise ValueError(f"Extension type {extension} not understood") except Exception as e: output_path = f"{os.path.splitext(output_path)[0]}.pkl" safe_file_dump(self, output_path, "dill") logger.error( "\n\nSaving the data has failed with the following message:\n" - "{}\nData has been dumped to {}.\n\n".format(e, output_path) + f"{e}\nData has been dumped to {output_path}.\n\n" ) def save_posterior_samples(self, filename=None, outdir=None, label=None): diff --git a/test/core/result_test.py b/test/core/result_test.py index abef90dde..cbe6ec6d0 100644 --- a/test/core/result_test.py +++ b/test/core/result_test.py @@ -874,6 +874,49 @@ def test_reweight_different_likelihood_weights_correct(self): self.assertLess(abs(new.log_evidence - self.result.log_evidence), 0.05) self.assertNotEqual(new.log_evidence, self.result.log_evidence) + def test_save_to_file_filename_with_extension_and_extension_none(self): + # Should use the extension from filename + filename = os.path.join(self.result.outdir, "custom_name.hdf5") + self.result.save_to_file(filename=filename, extension=None) + self.assertTrue(os.path.isfile(filename)) + os.remove(filename) + + def test_save_to_file_filename_with_extension_and_extension_true(self): + """This is a strange default, that we should remove, but is here for consistency""" + filename = os.path.join(self.result.outdir, "custom_name.hdf5") + expected = os.path.join(self.result.outdir, "custom_name.json") + self.result.save_to_file(filename=filename, extension=True) + self.assertTrue(os.path.isfile(expected)) + self.assertFalse(os.path.isfile(filename)) + os.remove(expected) + + def test_save_to_file_filename_with_extension_and_extension_set(self): + # Should override the extension in filename with the one provided in extension + filename = os.path.join(self.result.outdir, "custom_name.hdf5") + expected = os.path.join(self.result.outdir, "custom_name.json") + self.result.save_to_file(filename=filename, extension="json") + self.assertTrue(os.path.isfile(expected)) + self.assertFalse(os.path.isfile(filename)) + os.remove(expected) + + def test_save_to_file_filename_without_extension_and_extension_none(self): + # Should use the default extension (json) + filename = os.path.join(self.result.outdir, "custom_name_noext") + expected = filename + ".json" + self.result.save_to_file(filename=filename, extension=None) + self.assertTrue(os.path.isfile(expected)) + self.assertFalse(os.path.isfile(filename)) + os.remove(expected) + + def test_save_to_file_defaults_to_pickle_with_incorrect_extension(self): + """This is a weird fallback...""" + filename = os.path.join(self.result.outdir, "custom_name_noext") + expected = filename + ".pkl" + self.result.save_to_file(filename=filename, extension="bar") + self.assertTrue(os.path.isfile(expected)) + self.assertFalse(os.path.isfile(filename)) + os.remove(expected) + if __name__ == "__main__": unittest.main()