Skip to content

Support additional types in csv and fixes #102

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 27 commits into from
Mar 28, 2020
Merged

Support additional types in csv and fixes #102

merged 27 commits into from
Mar 28, 2020

Conversation

markaren
Copy link
Member

@markaren markaren commented Mar 25, 2020 •
edited
Loading

  • Support additional FMI types in CSV. Declared in header. Defaults to real.
  • Fix index stepper logic

@markaren
Copy link
Member Author

@fcollonval Have a look if you want/have time in case there are obvious things that could be done more pythonic.

markaren
markaren commented Mar 26, 2020
View reviewed changes
fcollonval
fcollonval reviewed Mar 27, 2020
View reviewed changes
Copy link
Collaborator

@fcollonval fcollonval left a comment

Choose a reason for hiding this comment

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

@markaren I made a few comments and suggestions.

@markaren
Copy link
Member Author

PR should be ready. Some stuff I did not originally meant to target for this PR was also added to make stuff work.

fcollonval
fcollonval reviewed Mar 27, 2020
View reviewed changes
Copy link
Collaborator

@fcollonval fcollonval left a comment

Choose a reason for hiding this comment

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

I would document register_variable to explain the nested argument.

It would be nice to get an example for csv to play with in the demo (could be another PR).

@markaren @fcollonval
Document register_variable
4030388 
Co-Authored-By: Frédéric Collonval <[email protected]>
fcollonval
fcollonval approved these changes Mar 28, 2020
View reviewed changes
Copy link
Collaborator

@fcollonval fcollonval left a comment

Choose a reason for hiding this comment

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

LGTM

@fcollonval
Copy link
Collaborator

I went ahead and merge this to finalize a PR using subcommands.

@fcollonval fcollonval merged commit 14089e8 into master Mar 28, 2020
@fcollonval fcollonval deleted the csv-improvements branch March 30, 2020 12:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@fcollonval fcollonval fcollonval approved these changes

Assignees
No one assigned
Labels
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@markaren @fcollonval