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

Land ice mesh tools to python 3 #249

Merged
merged 2 commits into from
May 8, 2019

Conversation

xylar
Copy link
Collaborator

@xylar xylar commented Mar 2, 2019

Make a few minor formatting changes to the land ice mesh tools so they work in python 3 as well as python 2.

I also include mark_horns_for_culling.py because I think this script might be used primarily by the land-ice model and because I'm pretty sure that @matthewhoffman is the author of all 5 scripts.

@xylar xylar self-assigned this Mar 2, 2019
@xylar xylar changed the title Land ice mesh tools to python3 Land ice mesh tools to python 3 Mar 2, 2019
@xylar
Copy link
Collaborator Author

xylar commented Mar 2, 2019

@matthewhoffman, here are changes needed to make the land ice mesh tools compatible with python 3. This should give you a sense of some of the more basic changes that are typically required. Mostly about print formatting.

A few of the formatting changes aren't strictly necessary but there's a strong preference for the .format() as opposed to % or + and I was having to alter print statements all over in any case. If you feel strongly about those changes, I can obviously revert them.

@xylar
Copy link
Collaborator Author

xylar commented Mar 2, 2019

I don't have a great way to test these. I made sure they pass the --help test and that's about it.

@xylar
Copy link
Collaborator Author

xylar commented Mar 4, 2019

The intention is to revise these scripts before including them in the mpas_mesh_tools conda package (#248)

@matthewhoffman
Copy link
Member

@xylar , thanks for putting this together - it looks like a lot of tedious work. I'll take a look and try the new versions out.

@xylar
Copy link
Collaborator Author

xylar commented Mar 5, 2019

@matthewhoffman, eh, not as much work as I was anticipating. I'm starting to learn that a lot of the difficulties between python 2 and 3 come when you use fancier features like dictionaries and parse config files and stuff like that that you really only do when putting packages together, not scripts. With scripts, it's almost entirely the print statements.

@xylar
Copy link
Collaborator Author

xylar commented Mar 5, 2019

Let me know if I should split mark_horns_for_culling.py into a separate PR.

@xylar xylar force-pushed the land_ice_mesh_tools_to_python3 branch from 64299cf to 68749f4 Compare March 16, 2019 04:07
@xylar xylar added the python 3 label May 5, 2019
@xylar xylar force-pushed the land_ice_mesh_tools_to_python3 branch from 68749f4 to 3035a37 Compare May 5, 2019 15:49
@xylar
Copy link
Collaborator Author

xylar commented May 5, 2019

@matthewhoffman, could you suggest a process I could use to test these myself? I would like to include them in #248, which I think @mark-petersen and I will work on getting merged this week.

@xylar xylar force-pushed the land_ice_mesh_tools_to_python3 branch from 3035a37 to f00466a Compare May 6, 2019 06:02
@xylar
Copy link
Collaborator Author

xylar commented May 8, 2019

@matthewhoffman, @mark-petersen and I want to merge #248 today if at all possible. This builds off of this PR. I think I want to take the small risk that I broke your scripts when I added python 3 support and just merge these changes unless you want to test them today.

The alternative would be that I remove these scripts from the conda package. Which would you prefer?

@mark-petersen
Copy link
Collaborator

My two cents: Let's take the leap on these and clean up any loose ends afterwards. @xylar has taken all reasonable precautions to change over to python 3.

@matthewhoffman
Copy link
Member

@xylar , that sounds entirely reasonable to me. I apologize for my failing to give this the attention it deserves.

@xylar
Copy link
Collaborator Author

xylar commented May 8, 2019

Thanks @matthewhoffman.

@xylar xylar merged commit fed2fdc into MPAS-Dev:master May 8, 2019
@xylar xylar deleted the land_ice_mesh_tools_to_python3 branch May 8, 2019 15:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants