Python 3 compatiability fixes; measurement result example changes

[miguelcleon]

This PR upgrades WOFpy for Python 3. I've upgraded ODM2 Admin for Python 3 and I wanted to run WOFpy on the same server and I didn't want to run web applications with different versions of python.

I'm not sure if you will want to merge this but I thought I'd make the PR so we can discuss.

I have this running with python 3 here http://bitnami-miguel.cuahsi.org/odm2lczo/

This PR also contains updates for the examples/flask/odm2/measurement DOA and related files. The previous version did not work with an example database water quality database I've been working with. This is currently deployed here https://dev-odm2admin.cuahsi.org/ansp/ @emiliom let me know what you think, Maybe @lsetiawan can look at this or someone else?

[aufdenkampe]

@miguelcleon, thank you for doing this! Moving the ODM2 stack to Python 3 compatibility has long been on our TO DO list, and this PR gets us one step closer. Also, I think now is finally the right time to make this transition for lots of reasons. See https://github.com/ODM2/ODM2PythonAPI/issues/157#issuecomment-420043196 for Python 3 discussion regarding the ODM2PythonAPI.

I see that this PR is failing the Travis tests. Ideally, we could issue a release that is compatible with both Python 2 & 3. How easy is that? Does a compatibility layer like this help: https://python-future.org?

I added @emiliom and @ocefpaf as reviewers.

[aufdenkampe]

@ocefpaf, thanks for those suggestions. I ideally, we would go 100% Python 3, but unfortunately, we can't do that for WOFpy until the https://github.com/ODM2/ODM2DataSharingPortal repo moves to Python 3. @horsburgh, @jcaraballo17 and @Maurier, what would it take to do that? Is a transition to Python 3 something that we might put on the roadmap for the ODM2DataSharingPortal?

I think that would be the main constraint, as we could simultaneously move WOFpy and ODM2API to Python 3.

[aufdenkampe]

I should add that existing deployments and examples, of which there are many, could continue to point to our older releases.

However, the safest plan would be to maintain backward compatibly with Python 2.7 for at least a year or so, if it's not too much work.

It was helpful for me to read this as I considered the effort: https://docs.python.org/2/howto/pyporting.html A key point from that is that "modernizing your Python 2 code to also support Python 3 is largely automated for you."

cc: @steveskrip

[horsburgh]

@aufdenkampe - we are using a bunch of Python 2 libraries for the data sharing portal. So, conversion to Python 3 is a good goal, but we'll have to look more closely at the effort involved. But, I don't think it's necessary to tie these two together. WOFPy is a completely separate application. We are running it in a separate Python Virtual Environment on the Data Sharing Portal server, so it's isolated in terms of Python.

I think you could go ahead and upgrade WOFPy to Python 3 and then the virtual environment within which it is running on the data sharing portal server could be updated. It shouldn't affect the data sharing portal application.

[emiliom]

Sorry for not replying yet; I've been under a couple of hard deadlines -- and still am. @miguelcleon, thank you!! This is very cool. I'll go over this later, and start commenting by tomorrow or so.

[emiliom]

Ok. Sorry it took me such a long time to follow up. Again, thanks so much @miguelcleon !! And I'm really glad you got your ODM2 measurement results working! @ocefpaf and @aufdenkampe, thanks for your follow ups!

Let's get a couple of things out of the way:

• ODM2 Data Sharing Portal: As @horsburgh said, that's a separate application with not API-level linkage. So, any discussion of Py3k upgrade for the ODM2 Sharing Portal should happen independently and in its own repo.
• This PR combines two substantial and separate feature upgrades: Py3k and fixing the ODM2 measurement results DAO/example to work with Miguel's LCZO database. Let's break them up into separate PR's. @miguelcleon, I realize you probably did the two feature enhancements at the same time, though, so I realize it might be tricky to separate them -- though this PR is made up of several commits, which makes me hopeful. Anyway, I've created two new issues to track those two features: Py3k and ODM2 measurement results DAO.

Py3k

I have a hunch the Py3k upgrade will be trickier and will require broader discussions and PR's beyond this one, so having an umbrella issue will help. FYI, on that issue I've linked to the resources pointed out on this PR comments, plus others.

As mentioned on that issue, we had already done some Py3k-oriented code changes over a year ago. But we have done no focused testing whatsoever. As @ocefpaf mentioned, all tests in the CI are Py2 based (ie, based on a Python 2.7 conda env).

I can't guarantee that I have the time to see Py3k changes to completion, including tests. We should probably create a new branch for this development. @ocefpaf, would you be able to help or guide me with this? I don't think I have time to do it this week, but I can start discussing it.

@miguelcleon:

• I'm somewhat surprised your changes don't cover more code files ...
• What Py3 testing have you done?
• The only ODM2 Python dependency WOFpy uses is odm2api, which hasn't been fully upgraded (or tested) to Py3. Obviously there's no Py3 odm2api conda or pypi pip package. So, did you build odm2api "manually"? WOFpy's use of odm2api is pretty narrow and limited, so it's very possible that those components are indeed already Py3 ready, but I'd be surprised if a Py3 pip package can be built w/o failures
• My understanding is that spyne (a WOFpy dependency) is not well confirmed as being Py3 ready, specially Py3.5+. Where are you grabbing that dependency from?
• I see that in fixing the ODM2 measurement results DAO, you ended up copying lower-level WOFpy code into the DAO folder, examples/flask/odm2/measurement. Specifically: core.py, core_1_1.py, /core_1_0.py, dao.py, apps/spyned_1_1.py. Fixing this may be a simple matter of moving these files to where they belong, hopefully. But this situation also means that currently only the ODM2 measurement results DAO is using this new code, while the ODM2 timeseries results DAO is using the largely unmodified code (though you also changed wof/core.py, which may conflict with your changes to examples/flask/odm2/measurement/core.py)

ODM2 Measurements Results DAO/Example

@miguelcleon, if it's not too much trouble, can you list out in the new issue I created, #232, the main changes you made to make this work, besides Py3k changes? That'll help me as I review the commit(s). Ideally, I'd prefer to first address a PR focusing on this feature, and leave Py3k components for later.

[ocefpaf]

I agree with everything that @emiliom said in https://github.com/ODM2/WOFpy/pull/230#issuecomment-444238806 but this part:

We should probably create a new branch for this development.

By creating a long lived dev branch you may create huge merge conflicts down the road. There is nice a Python talk on how Instagram move its huge code base to Python 3 without long lived dev branches and by merging it into master! Worth watching (or reading the blog post about it).

IMO the plan should be:

1. identify the dependencies that must be migrated to Python 3 and start converting them from the bottom up in the dependency chain;
2. start hard failures for Python 3 in the CIs so new code is always compliant;
3. choose a migration technique: future, six, 2to3, a combination of them, or even Python 3 only code from now on.

[emiliom]

I agree with everything that @emiliom said in #230 (comment) but this part:

We should probably create a new branch for this development.

By creating a long lived dev branch you may create huge merge conflicts down the road.

Thanks @ocefpaf! I was hesitating as I wrote that. I completely agree with the plan you laid out. So, I'll copy it over to the Py3k issue :smile_cat:

[roelofversteeg]

Hi All

I have been lurking here, but I wanted to put in my $0.02. As Anthony has mentioned, we have in my company been working on using ODM2.

As part of this effort we have migrated the odm2api to python 3 and also made a number of changes to the ODM2 schema (part of these are listed as ODM2.1 on github). Many of the changes to the schema were things that people discussed before and were posted in the different documents. Others are ones that we realized were needed (for instance, the ability to store multiple numbers associated with a geochemical sample analysis, rather than have the single result -> resultvalue -> resultmeasurementvalue. (for instance measurement and std dev, or min and max). We extended and fixed some bugs in the odm2api.

We have been testing this on a fairly large multi year, multi site, multi sample geochemical dataset. This contains about 4000 discrete samples from 20 locations, with a total of > 100K analytical results. This allowed us to really test performance and so on.

We decided not to try to take any of the other capabilities along in this migration (so no WofPy, YAML, Django web stack and so on). As these all depend on the old schema, they would be impacted by this.

The changes to the ODM2 schema and to the api are both big and small. I want to share them with the community, and plan to do so after the AGU, but I think this is looking more and more like something which will become a fork, rather than something which will be easy to merge, as adopting this for a new odmxx database is fine, but making it work with an older one will be challenging.

Not sure how to proceed on this, but just wanted to start the discussion. My suggestion is to get together with whomever is there at the AGU next week, I can show what we have done, and then see how the community thinks I should proceed. I am there all week, with the only firm commitment my talk at 430 pm on Friday

Regards

Roelof

Regards

Roelof

On Tue, Dec 4, 2018 at 3:19 PM Filipe notifications@github.com wrote:

I agree with everything that @emiliom https://github.com/emiliom said in #230 (comment) https://github.com/ODM2/WOFpy/pull/230#issuecomment-444238806 but this part:

We should probably create a new branch for this development.

By creating a long lived dev branch you may create huge merge conflicts down the road. There is nice a Python talk https://www.youtube.com/watch?v=66XoCk79kjM on how Instagram move its huge code base to Python 3 without long lived dev branches and by merging it into master! Worth watching (or reading the blog post about it).

IMO the plan should be:

1. identify the dependencies that must be migrated to Python 3 and start converting them from the bottom up in the dependency chain;
2. start hard failures for Python 3 in the CIs so new code is always compliant;
3. choose a migration technique: future, six, 2to3, a combination of them, or even Python 3 only code from now on.

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/ODM2/WOFpy/pull/230#issuecomment-444243308, or mute the thread https://github.com/notifications/unsubscribe-auth/AHjhtMxPRwz2jEplArR_hK-dqgGf-bWGks5u1tjQgaJpZM4Y3xzV .

[emiliom]

Thanks for the update, @roelofversteeg. I'm looking forward to hearing more details about the work you describe. I won't be at AGU, and I can't remember if Anthony will.

I'll be interested in following up on your work updating your odm2api fork to Python 3, but that'll have to wait till January.

[emiliom]

@miguelcleon, I'm getting back to this PR (and WOFpy, more generally). Can you respond to my long comments from 12/4? Thanks!

[miguelcleon]

@emiliom sorry I haven't gotten back to this. Unfortunately I have some higher priority things to work on right now and I'm not sure when I'm going to be able to get to this.

[emiliom]

@miguelcleon Understood. Thanks for letting me know. I'll see what I can do on my own this month, since I'll be working on WOFpy.

[miguelcleon]

The latest commit, today adds a number of additional changes for python 3 compatibility.

@emiliom

I'm somewhat surprised your changes don't cover more code files ... What Py3 testing have you done?

I had previously only tested WOF with ODM2 time series and did not attempt to apply changes to the entire WOFpy code base. However now I have applied the same changes needed for ODM2 time series to the entire code base.

This has not been thoroughly tested. I'm using my fork for production though, so you all can decide to merge this or not.

[emiliom]

@miguelcleon could you comment on the last 3 of my 5 comments under the "Py3k" heading, above? Namely: odm2api and spyne dependencies in a Py3k environment; and the comment that starts with "I see that in fixing the ODM2 measurement results DAO, you ended up copying lower-level WOFpy code into the DAO folder, examples/flask/odm2/measurement." Thanks.