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

Reading csvy model causes problems using Configuration #1294

Closed
wkerzendorf opened this issue Sep 4, 2020 · 2 comments · Fixed by #1559
Closed

Reading csvy model causes problems using Configuration #1294

wkerzendorf opened this issue Sep 4, 2020 · 2 comments · Fixed by #1559

Comments

@wkerzendorf
Copy link
Member

wkerzendorf commented Sep 4, 2020

Code Sample, a copy-pastable example if possible

config = Configuration.from_yaml('blondin_model_compare_ddc10.yml')
run_tardis(config)

Problem description

This produces an error about v_inner_boundary not allowed to be defined. Just using the file directly works.

Added by Jordi:

Using a similar procedure with a different file produces a different error:

config = Configuration.from_yaml('tardis_csvy_example.yml')
run_tardis(config)

jsonschema.exceptions.ValidationError: Additional properties are not allowed ('config_dirname' was unexpected)

If the dictionary is being generated by TARDIS itself this shouldn't be a problem.
I'm looking into both problems

@ebjordi
Copy link
Contributor

ebjordi commented Sep 4, 2020

I'm looking into it

@ebjordi
Copy link
Contributor

ebjordi commented Sep 4, 2020

when you run run_tardis(config) TARDIS tries to create a Configuration object from that config arg without verifying if it's already one, it considers that is a filename or a simple dict:

     36         tardis_config = Configuration.from_yaml(config)
     37     except TypeError:
---> 38         tardis_config = Configuration.from_config_dict(config)

and checking Configuration.from_yaml it end up using Configuration.from_config_dict

    def from_yaml(cls, fname, *args, **kwargs):
        try:
            yaml_dict = yaml_load_file(
                fname, loader=kwargs.pop("loader", YAMLLoader)
            )
        except IOError as e:
            logger.critical(f"No config file named: {fname}")
            raise e

        tardis_config_version = yaml_dict.get("tardis_config_version", None)
        if tardis_config_version != "v1.0":
            raise ConfigurationError(
                "Currently only tardis_config_version v1.0 supported"
            )

        kwargs["config_dirname"] = os.path.dirname(fname)

        return cls.from_config_dict(yaml_dict, *args, **kwargs)

so the implementation becomes redundant, maybe the solution would be to add a verification before trying to make a new Configuration object?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants