Skip to content

Conversation

@bjlittle
Copy link
Member

No description provided.

@pelson
Copy link
Member

pelson commented Nov 20, 2012

A brief review suggests this looks good to go.
I know @MartinDix is interested in this functionality. Martin - are you in a position to try this out on your own data?

Cheers,

@pelson
Copy link
Member

pelson commented Nov 20, 2012

I know @MartinDix is interested in this functionality.

I might be wrong actually. Before @MartinDix puts any effort into this, @bjlittle do you think this PR addresses #185?

@rhattersley
Copy link
Member

I might be wrong actually. Before @MartinDix puts any effort into this, @bjlittle do you think this PR addresses #185?

It does address part of the problem mentioned (namely LBPACK being 0 or 2 for a 64-bit fieldsfile), but it doesn't tackle 32-bit fieldsfiles .... that's the part @MartinDix has offered to tackle once this PR is merged.

@rhattersley
Copy link
Member

As yet I've only given it a quick skim, but it looks like an excellent change. Just the kind of thing I was hoping for. :-)

And as I think it'll be the first PR to introduce the mock module, could you add it to the INSTALL file please.

Copy link
Member

Choose a reason for hiding this comment

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

Long line.

@ghost ghost assigned rhattersley Nov 21, 2012
Copy link
Member

Choose a reason for hiding this comment

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

There are quite a few long lines in this method.

Copy link
Member

Choose a reason for hiding this comment

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

Should the use of "pp" (e.g. in "_pp_payload", "pp_field", and the like) be removed? Is this just a hangover from copying the pp module?

@rhattersley
Copy link
Member

Some minor formatting and refactoring... but otherwise lookin' good, and I'm itching to get it merged. :-)

@MartinDix
Copy link
Contributor

This works properly with our 64 bit files with LBPACK=0 and 2. Thanks.

rhattersley added a commit that referenced this pull request Nov 22, 2012
Fieldsfile and PP support for unpacked and CRAY 32-bit packed data.
@rhattersley rhattersley merged commit 9356661 into SciTools:master Nov 22, 2012
@rhattersley
Copy link
Member

This works properly with our 64 bit files with LBPACK=0 and 2. Thanks.

Thanks for the feedback :-)

@pelson
Copy link
Member

pelson commented Nov 22, 2012

👍 good work @bjlittle. Thanks for testing @MartinDix.

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

Successfully merging this pull request may close these issues.

4 participants