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

Improvement of parse function #1

Open
PatrikHlobil opened this issue Apr 26, 2018 · 3 comments
Open

Improvement of parse function #1

PatrikHlobil opened this issue Apr 26, 2018 · 3 comments

Comments

@PatrikHlobil
Copy link
Contributor

Hi Rafal,

I have to say that your visum_to_pandas script is great. I worked a bit on the most recent version and made the following changes/improvement for the "parse" function:

  • use os.path.join for the combination of paths
  • use "string".split(9 to get rid of newline and space characters
  • auto-detect COL_SEPARATOR
  • do not export index of DataFrame
  • add parameters (export_path, export) to the parse function:

:param export_path (optional): Path to folder, where to save the CSV files.
If not specified, the CSV files are save in
folder of the net files.
:param export (optional): List of network objects that should be exported. If
not specified, all network objects will be exported.

If you like, you can just include my changes in your code. The file was uploaded with the name "ptv_visum_to_pandas_2.py".

Best regards,

Patrik

@RafalKucharskiPK
Copy link
Owner

Hi Patrik,

thanks for commitment. Your imporovements are nice, I appreciate.
Please try to create a pull request so that we can merge.

  • I'd like to still modify some of the code, e.g. we do not need have twice calls to pd.DataFrame() after if export != None:
  • I'd like to check if your 'COL_SEPARATOR' identification works for OSX also (since I use it a lot).
  • It'd be nice to have a list of tables available to export prepared in code, so that user can refer to it.
  • the paths and params of the script, they shall be ultimately unified into some .json config or sth similar - so that maybe you idea of passing path as param maybe not needed.

Thanks again
Rafał

@PatrikHlobil
Copy link
Contributor Author

Hi Rafal,

I created a branch and a pull request (this is my first contribution on GitHub, so still learning how all that works : D ).

How exactly do you think users should use the script? I don't understand why you want to use a .json config in combination with the script? I thought that it should serve as a module, from which you can load the functions when needed. In this case, one has to specify the parameters that I introduced. What are your thought on this?

Best,
Patrik

@RafalKucharskiPK
Copy link
Owner

RafalKucharskiPK commented Apr 27, 2018 via email

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

No branches or pull requests

2 participants