diff --git a/src/huggingface_hub/hub_mixin.py b/src/huggingface_hub/hub_mixin.py index 86dce2dd54..fa9ee5e7bd 100644 --- a/src/huggingface_hub/hub_mixin.py +++ b/src/huggingface_hub/hub_mixin.py @@ -262,6 +262,12 @@ def save_pretrained( save_directory = Path(save_directory) save_directory.mkdir(parents=True, exist_ok=True) + # Remove config.json if already exists. After `_save_pretrained` we don't want to overwrite config.json + # as it might have been saved by the custom `_save_pretrained` already. However we do want to overwrite + # an existing config.json if it was not saved by `_save_pretrained`. + config_path = save_directory / CONFIG_NAME + config_path.unlink(missing_ok=True) + # save model weights/files (framework-specific) self._save_pretrained(save_directory) @@ -271,7 +277,6 @@ def save_pretrained( if config is not None: if is_dataclass(config): config = asdict(config) # type: ignore[arg-type] - config_path = save_directory / CONFIG_NAME if not config_path.exists(): config_str = json.dumps(config, sort_keys=True, indent=2) config_path.write_text(config_str) diff --git a/tests/test_hub_mixin.py b/tests/test_hub_mixin.py index ed137c2f3a..09dd91c56d 100644 --- a/tests/test_hub_mixin.py +++ b/tests/test_hub_mixin.py @@ -302,10 +302,29 @@ def test_push_to_hub(self): # Delete repo self._api.delete_repo(repo_id=repo_id) - def test_save_pretrained_do_not_overwrite_config(self): - """Regression test for https://github.com/huggingface/huggingface_hub/issues/2102.""" + def test_save_pretrained_do_not_overwrite_new_config(self): + """Regression test for https://github.com/huggingface/huggingface_hub/issues/2102. + + If `_from_pretrained` does save a config file, we should not overwrite it. + """ model = DummyModelSavingConfig() model.save_pretrained(self.cache_dir) # config.json is not overwritten with open(self.cache_dir / "config.json") as f: assert json.load(f) == {"custom_config": "custom_config"} + + def test_save_pretrained_does_overwrite_legacy_config(self): + """Regression test for https://github.com/huggingface/huggingface_hub/issues/2142. + + If a previously existing config file exists, it should be overwritten. + """ + # Something existing in the cache dir + (self.cache_dir / "config.json").write_text(json.dumps({"something_legacy": 123})) + + # Save model + model = DummyModelWithKwargs(a=1, b=2) + model.save_pretrained(self.cache_dir) + + # config.json IS overwritten + with open(self.cache_dir / "config.json") as f: + assert json.load(f) == {"a": 1, "b": 2} diff --git a/tests/test_hub_mixin_pytorch.py b/tests/test_hub_mixin_pytorch.py index 48511b08d7..317b0a005e 100644 --- a/tests/test_hub_mixin_pytorch.py +++ b/tests/test_hub_mixin_pytorch.py @@ -346,10 +346,10 @@ def forward(self, x): # Linear layers should share weights and biases in memory state_dict = reloaded.state_dict() - a_weight_ptr = state_dict["a.weight"].storage().data_ptr() - b_weight_ptr = state_dict["b.weight"].storage().data_ptr() - a_bias_ptr = state_dict["a.bias"].storage().data_ptr() - b_bias_ptr = state_dict["b.bias"].storage().data_ptr() + a_weight_ptr = state_dict["a.weight"].untyped_storage().data_ptr() + b_weight_ptr = state_dict["b.weight"].untyped_storage().data_ptr() + a_bias_ptr = state_dict["a.bias"].untyped_storage().data_ptr() + b_bias_ptr = state_dict["b.bias"].untyped_storage().data_ptr() assert a_weight_ptr == b_weight_ptr assert a_bias_ptr == b_bias_ptr