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

Python io rules #182

Closed
bblay opened this issue Oct 30, 2012 · 18 comments
Closed

Python io rules #182

bblay opened this issue Oct 30, 2012 · 18 comments

Comments

@bblay
Copy link
Contributor

bblay commented Oct 30, 2012

Rewrite the load and save rules as atomic python functions.

Consider creating guidelines on writing rule functions,

  • For example: atomic, containing single a if statement?.
  • grib_save_rules.py goes a long way to showing how this can be done.

It would be much easier to maintain and debug rules in this form.

Considerations:

  • ontology mapping project
  • will it actually provide worthwhile performance gains?
  • will this work for netcdf and future formats?
@bblay
Copy link
Contributor Author

bblay commented Oct 30, 2012

@marqh what is the view from the ontology translation project?

@pelson
Copy link
Member

pelson commented Oct 30, 2012

@bblay : I think it would help to share your motivation for this. Is it purely for performance?

@bblay
Copy link
Contributor Author

bblay commented Oct 30, 2012

It would be much easier to maintain and debug rules in this form.

(and performance)

@rhattersley
Copy link
Member

Duplicate of #118.

@bblay
Copy link
Contributor Author

bblay commented Nov 27, 2012

An important difference between rules files and python rules can be seen in #229, where individual python rules are being tested. This cannot be done satisfactorily with the rules files, which must be run as a whole.

This was referenced Dec 4, 2012
@bblay
Copy link
Contributor Author

bblay commented Dec 5, 2012

Another benefit not mentioned here yet:
The current rules system splits and duplicates much of the knowledge across multiple rules.
The python rules investigated in grib_save_rules.py attempt, and appear to, overcome this.

@cpelley
Copy link

cpelley commented Dec 17, 2012

Duplicate of #71

@bblay
Copy link
Contributor Author

bblay commented Dec 20, 2012

I don't think #71 is a duplicate, mentioning just one format and optimisation (not a change of implementation).

This ticket asks for all format loaders and savers to be written in Python,
until we have reason not to.

Readability and maintainability are the drivers here, with optimisation a possible, secondary side effect.

@cpelley cpelley mentioned this issue Apr 17, 2013
@bblay
Copy link
Contributor Author

bblay commented May 14, 2013

🐌 Bump.

I'd love this to move forward a little.
The longer we leave it the less manageable the rules become.

I'd be happy to rewrite the grib load rules in Python next time I have some free time.
Are there any objections to this idea?

@rhattersley
Copy link
Member

Are there any objections to this idea?

Well, there is the small matter of @marqh's metarelate project...

@bblay
Copy link
Contributor Author

bblay commented May 15, 2013

metarelate

That's just covering parameter translations for the foreseeable future, is it not?

It will be fantastic when it's giving us parameter translations but for the actual format translations, for quite some time at the very least, we seem to be stuck with decreasingly manageable rules files.

An attempt to move to Python would not only give us more manageable rules logic but also inform the metarelate project if it attempts to move into format translation. Win-win?

@bblay
Copy link
Contributor Author

bblay commented May 20, 2013

Wow, just returned to the grib load rules to add polar stereographic projection.

Looking at GribWrapper._compute_extra_keys it's clear we've already been forced down this route for much of the grib logic.

So, in part, moving to pure Python for the grib load rules would just be refactoring for maintainability.

@marqh
Copy link
Member

marqh commented May 21, 2013

An attempt to move to Python would not only give us more manageable rules logic but also inform the metarelate project if it attempts to move into format translation. Win-win?

I am not sure about this.

I am currently enmeshed in refactoring
fileformats/grib.py
and relocating much of the logic to enable grib save and grib load to encapsulate their logic clearly. I am hopeful that I will have a pilot implementation for this soon. As such, it could be somewhat confusing if there was a parallel activity to refactor the processing, particularly with respect to potential pull requests.

If effort is available, I think it would be better expended on scoping and refactoring grib.py than refactoring the grib rules.

@bblay
Copy link
Contributor Author

bblay commented May 22, 2013

it could be somewhat confusing if there was a parallel activity...

agreed, will wait for this.

@bblay
Copy link
Contributor Author

bblay commented May 23, 2013

I should add a comment: refactoring the Python code but keeping the rules files would be inadequate. The rules files have already become unacceptable. Look at the pp time saving rules, for example. They are almost unintelligable. Regarding the grib loading, the code has evolved to support the rules files so refactoring the code alone would just be rearranging deck chairs.

@bblay
Copy link
Contributor Author

bblay commented Jul 15, 2013

Some ammo in this discussion, where @rhattersley examines performance gains from Python rules.

@rhattersley
Copy link
Member

To be clear, I've just done a minimal syntax tweak to the existing rules. So there's just a single function in a new module which contains all the rules:

def convert(cube, f):
    ... all the rules go in here ...

And something like:

IF 
f.lbrsvd[3] != 0 
THEN 
CoordAndDims(DimCoord(f.lbrsvd[3], standard_name='realization'))

just becomes a snippet within that function:

    if f.lbrsvd[3] != 0:
        cube.add_aux_coord(DimCoord(f.lbrsvd[3], standard_name='realization'))

@bblay
Copy link
Contributor Author

bblay commented Jan 28, 2014

I has been agreed that translation rules will be written in Python.

@bblay bblay closed this as completed Jan 28, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants