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

Add export functionality to TOML in addition to JSON #327

Merged
merged 25 commits into from
Oct 10, 2024
Merged

Conversation

qubixes
Copy link
Member

@qubixes qubixes commented Sep 19, 2024

This also adds a new configuration TOML version: 1.1

Changes include that now defaults can be set for the data_free variables. For example, you don't need to type in the prop_missing for each variable anymore, you can set the default in the [defaults] section. You can also have default distributions now per variable type -> e.g. all continuous variables could by default be assigned the normal distributions.

TODO:

  • Documentation
  • Tests
  • Sample configuration files (also one file with all distributions).

Fixes #323 Fixes #322

@qubixes
Copy link
Member Author

qubixes commented Oct 1, 2024

@vankesteren I haven't completely finished yet, but it would be nice to get your input on the import/export functionality of MetaFrame. Currently we have mf.from_json, mf.to_json and mf.export. This works alright for json, however from toml this becomes very confusing, since we would have a mf.from_toml that would not take a configuration toml file. My proposal is to instead use mf.load and mf.store with suffix detection and mf.load_{suffix} and mf.store_{suffix} for direct load/store. The old methods will have a deprecation warning attached, but will still work for one or two minor versions of metasyn. What do you think?

@vankesteren
Copy link
Member

Agreed!

@vankesteren
Copy link
Member

maybe load and save instead of load and store, those names are a bit more common

@qubixes
Copy link
Member Author

qubixes commented Oct 2, 2024

Okay, I'll change it into load and save then, that sounds good to me as well.

README.md Outdated Show resolved Hide resolved
docs/source/developer/overview.rst Outdated Show resolved Hide resolved
@qubixes
Copy link
Member Author

qubixes commented Oct 4, 2024

@vankesteren I have finished this PR. I have edited the images, but I didn't find them all, so I have removed the few that I can't find. I have changed all Exported MetaFrame -> GMF file for consistency, the documentation itself also needs to change some at some point, but for now this is okay as far as I am concerned.

@qubixes qubixes marked this pull request as ready for review October 7, 2024 09:09
Copy link
Member

@vankesteren vankesteren left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fix the suggestions and then merge

.github/workflows/core-ci.yml Outdated Show resolved Hide resolved
metasyn/__main__.py Outdated Show resolved Hide resolved
Comment on lines +106 to +116
defaults = config_dict.pop("defaults", None)
privacy = config_dict.pop("privacy", None)
config_version = config_dict.pop("config_version", "1.0")
if config_version not in ["1.0", "1.1"]:
warnings.warn(f"Trying to read configuration file with version {config_version}, "
"this version of metasyn only supports 1.0 and 1.1.")
if privacy is not None:
if defaults is not None:
raise ValueError("Error parsing configuration file: cannot have both [privacy]"
" and [defaults] tables.")
defaults = {"privacy": privacy}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

At some point, we might want to change this to separate reader function for separate config versions.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

classes

self.save_json(fp, validate)
return
fp_path = Path(fp)
if fp_path.suffix == ".toml":
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

noice

qubixes and others added 2 commits October 10, 2024 10:48
Co-authored-by: Erik-Jan van Kesteren <e.vankesteren1@uu.nl>
Co-authored-by: Erik-Jan van Kesteren <e.vankesteren1@uu.nl>
@qubixes qubixes merged commit cd0e8ac into develop Oct 10, 2024
12 of 13 checks passed
@qubixes qubixes deleted the toml-gmf branch October 10, 2024 09:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Parsing of var_types Error messaging if toml file is not well formatted
2 participants