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

implement to_writer method and clean up RustCycle csv serde #95

Merged
merged 3 commits into from
Feb 6, 2024

Conversation

kylecarow
Copy link
Collaborator

@kylecarow kylecarow commented Jan 24, 2024

  • adds a to_writer method
    • RustCycle just needs to override to_writer instead of to_file
    • Cleanup/reorg of SerdeAPI-alike functions in RustCycle (write_csv etc)
    • New RustCycle::to_csv function to be consistent with to_yaml etc, same functionality as RustCycle::to_str("csv") (which now calls RustCycle::to_csv)
  • All SerdeAPI errors are now exposed as IOError/OSError in Python rather than RuntimeError

I noticed the Python-exposed RustCycle::from_csv loads a cycle from a given filepath, rather than a string like from_yaml, from_json, etc. I think this is something that I would definitely want to change in FASTSim 3. @calbaker what are your thoughts about making all these string deserialization methods have _str at the end (starting in FASTSim 3, to avoid API changes)?

i.e. from_yaml would become from_yaml_str to make it clear that it takes a YAML-formatted string, rather than a filepath

@calbaker
Copy link
Collaborator

@kylecarow , is this ready to merge?

@kylecarow
Copy link
Collaborator Author

@calbaker Left a comment for you to ponder, but this PR is ready to merge, yea

@calbaker calbaker merged commit 707d64e into fastsim-2 Feb 6, 2024
3 checks passed
@calbaker calbaker deleted the f2/to_writer branch February 6, 2024 19:35
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.

2 participants