Skip to content

New function cf.configuration with unit test #95

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.

Sign up for GitHub

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

Jump to bottom
Merged
sadielbartholomew merged 8 commits into from
Jul 13, 2020
Merged

New function cf.configuration with unit test #95

sadielbartholomew merged 8 commits into from
Jul 13, 2020

Conversation

@sadielbartholomew
Copy link
Member

@sadielbartholomew sadielbartholomew commented Jul 8, 2020 •
edited
Loading

Final step towards & therefore closes #70, after work to convert all functions to get & set individual global constants to lower-case was completed in #93. This is equivalent to NCAS-CMS/cfdm#61 but is more complicated as there are many more global constant settings to include for cf (13 vs. 3 for cfdm).

Opening as a WIP because in implementing this, particularly the unit test, I noticed that cf.atol & cf.rtol are detached from the corresponding constants in cf.CONSTANTS, instead getting & setting the cfdm equivalent (see #85 where I will detail it further). I would like to fix this first before completing this PR as it causes the unit test to fail (for valid reason). Best wait until the underlying issue is addressed to merge with a passing unit test (without bodging it temporarily).

Once that is discussed & resolved, I still want to extend the unit test for cf.configuration added here slightly, before removing the WIP status.

@sadielbartholomew sadielbartholomew added the enhancement New feature or request label Jul 8, 2020
@sadielbartholomew sadielbartholomew self-assigned this Jul 8, 2020
@sadielbartholomew sadielbartholomew mentioned this pull request Jul 8, 2020
Closed
@sadielbartholomew sadielbartholomew marked this pull request as ready for review July 13, 2020 21:29
@sadielbartholomew
Copy link
Member Author

sadielbartholomew commented Jul 13, 2020 •
edited
Loading

The CI is failing at the dependency stage, because the imminent version of cfdm is set as required, but that is not worth amending since the tests would also fail for reasons required to the latest netcdf_flattener not being packaged in a way that makes it practicable to install on the GH Actions job environment.

I ran the test suite locally and the only failure was for PEP8 compliance (not all relating to this PR) which I will address in a final commit:

...
test_Field_domain_axis_position (test_Field.FieldTest) ... ok
test_Field__mul__ (test_Field.FieldTest) ... ok

======================================================================
FAIL: test_pep8_compliance (test_style.styleTest)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/sadie/cf-python/cf/test/test_style.py", line 49, in test_pep8_compliance
    'Detected {!s} PEP8 errors or warnings:'.format(pep8_issues)
AssertionError: 66 != 0 : Detected 66 PEP8 errors or warnings:

----------------------------------------------------------------------
Ran 274 tests in 724.515s

FAILED (failures=1)

@sadielbartholomew
Copy link
Member Author

sadielbartholomew commented Jul 13, 2020

Local test suite now passes. Merging despite CI failures as justified above.

@sadielbartholomew sadielbartholomew merged commit e99e333 into NCAS-CMS:master Jul 13, 2020
@sadielbartholomew sadielbartholomew deleted the config branch July 13, 2020 22:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

@sadielbartholomew sadielbartholomew

Labels

enhancement New feature or request

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Consolidate functions that get & set global settings

1 participant

@sadielbartholomew