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

Xarray and other upgrades #75

Merged
merged 23 commits into from
Jul 8, 2019
Merged

Xarray and other upgrades #75

merged 23 commits into from
Jul 8, 2019

Conversation

jklenzing
Copy link
Member

@jklenzing jklenzing commented Jul 4, 2019

Description

Addresses #63 and #69.

  • Stores data from load routine as an xarray.Dataset.
  • Saves version and commit (short hash) info in 'version.txt' file archived with output files after model is run
  • Updates to documentation and setup procedures after use setup.cfg for metadata, build sami2py.x without Make #72.
    • cleans out unused test_suite structure
    • removes make command from travis
    • docstring updates
  • Updates to unit tests
    • Expanded tests to cover optional outputs
    • regenerated test data with 2 time steps and neutral info
    • Imposed upper limits on pandas and xarray to fix travis server
  • Minor fixes, including use of os.path.join

Type of change

  • Breaking change (data is now stored in a different format)
  • This change requires a documentation update

Context

This PR puts basic changes into play until f2py / netCDF is fully incorporated. Version info will eventually be stored in netCDF metadata.

Upper level limits on pandas and xarray not a permanent solution. Required for Travis CI to properly run python 2.7 tests.

Checklist:

  • Make sure you are merging into the develop (not master) branch
  • My code follows the style guidelines of this project
  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes
  • Any dependent changes have been merged and published in downstream modules

@jklenzing jklenzing added this to the 0.2.0 Release milestone Jul 4, 2019
@jklenzing jklenzing added this to To Do in User Interface Improvements via automation Jul 4, 2019
@jklenzing jklenzing added documentation Improve the description of the code enhancement New feature or request labels Jul 4, 2019
@jklenzing jklenzing moved this from To Do to Needs Review in User Interface Improvements Jul 4, 2019
@jklenzing jklenzing changed the title Xarray Xarray and other upgrades Jul 4, 2019
@scivision
Copy link
Contributor

scivision commented Jul 5, 2019

FYI,
I have turned away from using f2py in situations where more than one instance of the Fortran module (in this case, Sami2) needs to run at once OR if sequential runs are used. Common and module variables from one instance of the imported f2py Fortran code can modify those of another instance (or instances run sequentially have state leftover from first run), leading to non-deterministic (incorrect) results.

Maybe you all have run into and/or solved that issue. For me thus far, the solution was to not use f2py, and instead to pass I/O from Python to Fortran either directly via pipes (stdin/stdout), or if there was too much data for pipes then via tempfile.NamedTemporaryFile or similar (using RAM dist where possible).

setup.cfg Outdated
@@ -34,4 +34,6 @@ zip_safe = False
packages = find:
install_requires =
numpy
pandas<0.25
Copy link
Contributor

Choose a reason for hiding this comment

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

Could we add a comment here within setup.cfg so we don't forget it's just for Travis-I was puzzled till I saw the PR notes.

# FIXME: upper limit on pandas and xarray are just so Travis CI passes python 2.7 tests.

Copy link
Member Author

Choose a reason for hiding this comment

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

Good idea. Adding.

'-o', 'sami2py.x']
src = ['nrlmsise00_modified.f', 'grid-1.00.f', 'sami2py-1.00.f', 'hwm93.f', 'hwm07e_modified.f90',
'apexcord.f90', 'hwm14.f90']
cmd = ['gfortran', '-fno-range-check', '-fno-automatic',
Copy link
Contributor

Choose a reason for hiding this comment

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

This I think is fine for now. A more general approach for the future to consider is to have a sami2py.build() function that is automatically run on import sami2py if the Fortran module isn't built yet. I use that technique on my packages. This allows me to not depend on setuptools() and to switch compilers and options seamlessly/automatically.

But I think for now, probably most people have Gfortran and setuptools--just noting there's a more general way forward if it becomes an issue.

User Interface Improvements automation moved this from Needs Review to Approved Jul 8, 2019
@jklenzing jklenzing merged commit eb27ff1 into develop Jul 8, 2019
User Interface Improvements automation moved this from Approved to Done Jul 8, 2019
@jklenzing jklenzing deleted the xarray branch July 8, 2019 21:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improve the description of the code enhancement New feature or request
Development

Successfully merging this pull request may close these issues.

None yet

3 participants