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.

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

Fix bug that caused cf.Field.del_file_location to fail when updating its metdata constructs #708

Merged
merged 3 commits into from
Jan 17, 2024
Merged

Fix bug that caused cf.Field.del_file_location to fail when updating its metdata constructs #708

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
31c8b6f
reorder methods for clarity
davidhassell Jan 11, 2024
b530e0c
Fix bug in cf.Field.del_file_location
davidhassell Jan 11, 2024
a987100
Typo
davidhassell Jan 17, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions Changelog.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,12 @@
version 3.17.0
--------------

**2024-??-??**

* Fix bug that caused `cf.Field.del_file_location` to fail when
updating its metdata constructs
(http://github.com/NCAS-CMS/cf-python/issues/707)

version 3.16.0
--------------

Expand Down
2 changes: 1 addition & 1 deletion cf/field.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2543,7 +2543,7 @@ def del_file_location(

if constructs:
for c in self.constructs.filter_by_data(todict=True).values():
c.del_file_location(location, inplace=True)
c.del_file_location(location)

return location

Expand Down
22 changes: 22 additions & 0 deletions cf/test/test_Field.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2629,6 +2629,28 @@ def test_Field_subspace_ugrid(self):
self.assertTrue(g.aux("X").data.range() < 30)
self.assertTrue(g.aux("Y").data.range() < 50)

def test_Field_file_location(self):
f = cf.example_field(0)

self.assertEqual(f.add_file_location("/data/model/"), "/data/model")

cf.write(f, tmpfile)
f = cf.read(tmpfile)[0]
g = f.copy()
location = os.path.dirname(os.path.abspath(tmpfile))

self.assertEqual(f.file_locations(), set((location,)))
self.assertEqual(f.add_file_location("/data/model/"), "/data/model")
self.assertEqual(f.file_locations(), set((location, "/data/model")))

# Check that we haven't changed 'g'
self.assertEqual(g.file_locations(), set((location,)))

self.assertEqual(f.del_file_location("/data/model/"), "/data/model")
self.assertEqual(f.file_locations(), set((location,)))
f.del_file_location("/invalid")
self.assertEqual(f.file_locations(), set((location,)))


if __name__ == "__main__":
print("Run date:", datetime.datetime.now())
Expand Down
16 changes: 8 additions & 8 deletions docs/source/class/cf.Data.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -663,22 +663,22 @@ CFA
:toctree: ../method/
:template: method.rst

~cf.Data.file_locations
~cf.Data.del_file_location
~cf.Data.add_file_location
~cf.Data.cfa_clear_file_substitutions
~cf.Data.cfa_del_aggregated_data
~cf.Data.cfa_del_file_substitution
~cf.Data.cfa_file_substitutions
~cf.Data.cfa_update_file_substitutions
~cf.Data.cfa_del_file_substitution
~cf.Data.cfa_has_file_substitutions
~cf.Data.cfa_del_aggregated_data
~cf.Data.cfa_get_aggregated_data
~cf.Data.cfa_get_term
~cf.Data.cfa_get_write
~cf.Data.cfa_has_aggregated_data
~cf.Data.cfa_has_file_substitutions
~cf.Data.cfa_set_aggregated_data
~cf.Data.cfa_get_term
~cf.Data.cfa_get_write
~cf.Data.cfa_set_term
~cf.Data.cfa_set_write
~cf.Data.cfa_update_file_substitutions
~cf.Data.del_file_location
~cf.Data.file_locations

Element-wise arithmetic, bit and comparison operations
------------------------------------------------------
Expand Down
4 changes: 2 additions & 2 deletions docs/source/class/cf.Field.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -442,13 +442,13 @@ CFA
:toctree: ../method/
:template: method.rst

~cf.Field.file_locations
Copy link
Member

Choose a reason for hiding this comment

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

What (rough) ordering rules are we going for with respect to the API reference listings, e.g. here, for future reference (so I know what to try to abide by when adding new items myself, etc.)? It seemed to be alphabetical before but it appears that you are now preferring some loose form of categorisation based on the form of functionality?

Copy link
Collaborator Author

@davidhassell davidhassell Jan 17, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Hi Sadie - anarchy rules! Sometimes it seems that alphabetical makes sense, other times some logical grouping seems appropriate. When (no if!) we refactor the whole documentation we'll look at this in the round.

Copy link
Member

Choose a reason for hiding this comment

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

anarchy rules

I have that on a T-shirt, with an exclamation at the front (not really, but it would work 😄 )!

Fair enough (when AI takes over it won't be able to automate such rules, so good thinking 😁 ). In that case, I'll generally not touch anything myself as to change the order (not that I had plans to, this would only be when otherwise touching the API reference method or class listings) unless it looks obviously out of alphabetical or (some) grouping order. Thanks for clarifying.

~cf.Field.add_file_location
~cf.Field.del_file_location
~cf.Field.cfa_clear_file_substitutions
~cf.Field.cfa_del_file_substitution
~cf.Field.cfa_file_substitutions
~cf.Field.cfa_update_file_substitutions
~cf.Field.del_file_location
~cf.Field.file_locations

Geometries
^^^^^^^^^^
Expand Down
Loading