DAS-2232 - Functionality added to support SMAP L3 products #15

sudha-murthy · 2024-09-10T18:43:26Z

Description

SMAP L3 does not have 1D dimension scales and grid mapping variable which is needed by HOSS to do spatial subsetting.
Methods added to override the missing grid mapping with overrides in the hoss_config.json and supporting methods.
Methods also added to use the 2D coordinate datasets to generate 1D dimension scales that could be used for to calculate
the index ranges to provide the spatial subsetted outputs

Jira Issue ID

DAS-2232

Local Test Steps

Spatial subsetting can be tested without mask fill. (till DAS-2215 is complete)
Run Harmony in the box with HOSS.
Comment out of mask_fill in services.yaml file
Run a spatial subset request locally and ensure you get the right subsetted output
e.g.

3D variables do not work - pending on DAS-2238
New unit tests have not been added. The current unit tests are passing
DAS-2236 written to handle fill values in the corners.
Jupyter test notebooks exist for SMAP L3 and need to be updated

PR Acceptance Checklist

Jira ticket acceptance criteria met.
[X ] CHANGELOG.md updated to include high level summary of PR changes.
docker/service_version.txt updated if publishing a release.
Tests added/updated and passing.
Documentation updated (if needed).

hoss/spatial.py

hoss/dimension_utilities.py

…emoved duplicate method

…cate nethods

hoss/hoss_config.json

hoss/dimension_utilities.py

flamingbear

Sudha, There's a lot here to review and unfortunately I haven't worked with HOSS directly before. I tried to look through what I could and make decent suggestions.

But for sure, you need to test your changes. When you run the tests a coverage report is generated. Before your changes the results were:

Test Coverage Estimates
Name                           Stmts   Miss  Cover
--------------------------------------------------
Hoss/dimension_utilities.py      156      2    99%
hoss/spatial.py                   61      0   100%
--------------------------------------------------
TOTAL                            668      8    99%

And after they dropped considerably:


Test Coverage Estimates
Name                           Stmts   Miss  Cover
--------------------------------------------------
hoss/dimension_utilities.py      242     65    73%
hoss/spatial.py                   70      8    89%
--------------------------------------------------
TOTAL                            771     81    89%

It is very difficult to be able to understand the changes when I can't look at test and see what a function was supposed to do. Likewise, the function comments were not updated to describe the new functionality. Hopefully I'm not confusing anything with my lack of familiarity. I will defer to Owen if there's differences of opinion on what should be done.

Final test instructions assume a strong understanding of the problem you were solving, but I was able to eventually run the two test URLs and get output. Should the output files be geolocated somehow? Because they open in panoply, but aren't geo-2d, just 2d.

CHANGELOG.md

hoss/dimension_utilities.py

hoss/projection_utilities.py

hoss/spatial.py

owenlittlejohns

Thanks @sudha-murthy for putting a lot of effort into this PR!

This review is not 100% thorough. I think there are a few bigger things here that need to be addressed, and that will make it easier to digest in full:

I think the new functionality to deal with overriding dimensions should not just go in the get_projected_x_y_ranges. By doing so, it's adding complexity to that function, which makes things harder to understand overall. I also think it is forcing you to do some things that would otherwise be unnecessary - for example, I don't think you need to write the variables to the NetCDF-4 file, you just need to ask a function for 1-D dimension variables and spit out 1-D numpy arrays.
I'm really worried about the use of set objects to contain overriding dimensions. The ordering of dimensions is important, and must match the underlying array axes. I suspect this is the reason you are having issues with 3-D variables (it's certainly part of the issue).
A lot of the additional code is written in ways that adds high levels of cyclomatic complexity. I've suggested a few ways to cut that down, but it's worth taking a step back and working out what is common to code branches and what is truly different. Try to minimise what has to go in an if/elif/else, and then make sure each branch gives you the same output. The add_index_ranges function is perhaps the area of most concern.
There are a few places where the code branches and only assigns variables in one branch which are then used after the branching is finished. This will cause errors when something skips that block and later tries to access a non-existent variable.
This PR has no new or updated unit tests, either for new functions of new branches of code. I know I'm religious about these things, but they are absolutely needed. Particularly on a PR of this scope. There are a bunch of changes, and they are complicated and hard to understand. Not only do tests make sure things work, reading them makes it easier to understand how the code flows.
There are a lot of places where code has changed significantly, but the documentation strings describing the functions have not. They should definitely be updated to keep in sync with what's going on.

Sorry for all the comments, but hopefully they are helpful!

CHANGELOG.md

docker/service_version.txt

owenlittlejohns · 2024-09-20T20:05:26Z

hoss/dimension_utilities.py

@@ -55,6 +59,42 @@ def is_index_subset(message: Message) -> bool:
    )


+def get_override_projected_dimensions(


This PR does not include any changes to unit tests. Each new function you are adding (or new branch of code within an existing conditional block) needs unit testing that hits every branch of the code within a function. I've not reviewed any of the code changes in detail yet, but this lack of tests is an absolute blocker for me to approve the PR.

Yes. Owen. Will add the unit tests.

hoss/subset.py

hoss/dimension_utilities.py

owenlittlejohns · 2024-09-20T22:31:43Z

hoss/dimension_utilities.py

+    if variable.dimensions == []:
+        override_dimensions = get_override_dimensions(varinfo, [variable_name])
+        if len(override_dimensions) > 0:
+            for override in reversed(list(override_dimensions)):


Why are you reversing things here? The ordering should be preserved. If the order of override dimensions does not reflect the ordering of array axes, then that needs to be fixed.

I will revisit that. I had a problem with fixing the override dimensions list.

Changes are in commit 756f7c0

owenlittlejohns · 2024-09-20T22:40:47Z

hoss/dimension_utilities.py

@@ -422,22 +559,48 @@ def add_index_range(

    """


For this function: READ THIS COMMENT FIRST:

I think this is really overcomplicated. Fundamentally, all this function is doing is getting a list of dimensions for a variable. Then it is looking in a cache to see if there is an index range for any of the dimension names, before using that cache value to tack something on the end of a string.

Maybe I'm being naive, but it feels like really all you need is something to determine the correct list of variable dimensions, then all the rest of the logic (looking in the cache and appending strings to the end of the variable name) is the same. That latter stuff 100% does not need to be in the duplicated in multiple condition branches. It's making this function unnecessarily hard to read.

The other, really important comment: I am super suspicious of the bit where you are needing to reverse the order of the dimensions list. However that is derived, it should be reliably in the ordering of the variable axes. I'm wary that what this means is that you have found that for your particular use-case the "random" ordering in a set is predictable for the pseudo-dimensions you have for SMAP L3, and you can coincidentally impose the order you need coincidentally by reversing the set. I really think dimensions should not be passed around in sets, because you need that ordering. I strongly suspect this is the root cause of your issues with 3-D variables.

The ordering and the shape are things I need to get from varinfo. I dont have that information. Updated DAS-2241 for this.

Maybe rephrasing a different comment: I'm very uncomfortable with the thought of merging code with known problems into main. Going back to one of the things mentioned in the TRT breakout last week:

Teams ought to have a high-quality, maintainable baseline without known defects (particularly in new features) that is deployable to the production environment at all times.

Instead of making a massive PR that does 80% of a lot of stuff, this probably should be submitted piecemeal, with each piece being made 100% solid in a series of smaller PRs.

If you and David feel we're at a point that we can't break the changes down in that way, then a compromise might be to update this PR to merge into a feature branch. Then once all of the multiple PRs are merged into the feature branch, the feature branch can be merged into main.

I think terminology has confused some of this discussion. The question of reversing is not one of reversing dimensions, but the lat/lon variables coming from the Coordinates attribute. The recommendation here is that the get_coordinates method itself should return in a standard order (reversed in case), based upon the variable name - and not using reverse as shown here.

I don't think this is a case of "Known Issue".

We are planning to release a version for NSIDC to start testing with, which may not handle the 3D cases or other issues, but this release should not break any existing use cases, and should not truly break, but simply not handle as desired. Incremental feature release is a tenet of agile development.

Changes are in commit 756f7c0

I have simplified the method. The order for SMAP is 'projected_y', 'projected_x'. The override section of the code is only used by SMAP at the moment. It can be generalized if we can get that order of dimensions from varinfo. I am not sure if the order of dimensions is used for other products currently handled by hoss.

The use of sets for required_dimensions is based on how it is returned by varinfo and how it was originally in hoss before my changes. the bounds update requires the dimensions to be a set , It fails for a list

There's a difference between a set of all dimensions for all required variables (as used in the prefetch function), which aggregates all Variable.dimensions attributes (which individually are lists), and the dimensions on an individual variable. Variable.dimensions is not a set for ordering purposes, it is a list.

With regards to bounds - we know that the pseudo-dimensions won't have a bounds attribute, so you might be better to not try to find bounds variables for them. Then you'll avoid any compatibility issues there.

The add_index_range is a lot simpler now

hoss/dimension_utilities.py

owenlittlejohns · 2024-09-24T21:27:02Z

A couple of quick thoughts:

autydp yesterday
The review is a bit disjointed now with all the comments.

I agree that this PR is busy with a lot of comment. I'd recommend that @sudha-murthy picks a few of the comments in the PR to address at a time, pushes a commit up with them, and indicates in the comments that she has addressed them (with a link to the commit). Then the person making the comment can decide whether to mark it as resolved. Resolving the comments will make the PR look a lot less cluttered, and allow us to all focus on the remaining pieces.

flamingbear 4 hours ago
But since it looks like you've thumbed up a bunch of things, you can add a comment with the githash as a way to alert the commenter that you have addressed the issue, when you get there.

Yup - agreed.

CHANGELOG.md

…ables

owenlittlejohns

This is another incomplete review, but there are some things raised that I think are important to address:

Coping with multiple grids.
I genuinely think the method to get the 1-D grid dimension variables is far too complicated. (You could cut out probably 100 lines of code by just using pyproj)

hoss/dimension_utilities.py

owenlittlejohns · 2024-10-02T20:47:20Z

hoss/dimension_utilities.py

+        if not contains_latitude:
+            raise MissingCoordinateDataset('latitude')
+        if not contains_longitude:
+            raise MissingCoordinateDataset('longitude')


This function feels pretty convoluted now. I think it might be easier to do:

coordinate_variables_set = varinfo.get_references_for_attribute( requested_variables, 'coordinates' ) latitude_coordinate_variables = [ coordinate for coordinate in coordinate_variables_set if varinfo.get_variable(coordinate).is_latitude() ] longitude_coordinate_variables = [ coordinate for coordinate in coordinate_variables_set if varinfo.get_variable(coordinate).is_longitude() ] if not latitude_coordinate_variables: raise MissingCoordinateDataset('latitude') if not longitude_coordinate_variables: raise MissingCoordinateDataset('longitude') return latitude_coordinate_variables, longitude_coordinate_variables

This reduces the number of local variables in the function, reduces the cyclomatic complexity and uses the more Python-native model of list-comprehensions. (Minor downside is two list comprehensions looping through the coordinates, but they should be a handful of elements, so I don't think we need to worry about performance)

One other thought, though, what if the metadata attributes for the collection point to non existent variables? That will raise an exception that you currently aren't capturing.

Another thought (probably more important) - what about multiple grids? SPL3FP has global and polar grids, so has 2 latitude and 2 longitude coordinate variables. Your current implementation would get you something that is a 4-element list, and that ordering would depend entirely on the random ordering of the output of VarInfoFromDmr.get_references_for_attribute. That seems troubling. A big question here is how to make sure that the right latitudes and longitudes are being paired together.

When you write the unit tests for a lot of these functions, I definitely want to see what happens for a use-case when variables from different grids are being requested. We're going to need those tests to make sure things don't break.

Good point. will add the multiple grid test cases

This function feels pretty convoluted now. I think it might be easier to do:

coordinate_variables_set = varinfo.get_references_for_attribute( requested_variables, 'coordinates' ) latitude_coordinate_variables = [ coordinate for coordinate in coordinate_variables_set if varinfo.get_variable(coordinate).is_latitude() ] longitude_coordinate_variables = [ coordinate for coordinate in coordinate_variables_set if varinfo.get_variable(coordinate).is_longitude() ] if not latitude_coordinate_variables: raise MissingCoordinateDataset('latitude') if not longitude_coordinate_variables: raise MissingCoordinateDataset('longitude') return latitude_coordinate_variables, longitude_coordinate_variables

This reduces the number of local variables in the function, reduces the cyclomatic complexity and uses the more Python-native model of list-comprehensions. (Minor downside is two list comprehensions looping through the coordinates, but they should be a handful of elements, so I don't think we need to worry about performance)

updated the method - 802fe0e

#15 (comment)

not sure what that is referring to in the earlier comment

One other thought, though, what if the metadata attributes for the collection point to non existent variables? That will raise an exception that you currently aren't capturing.

if the coordinate variables don't exist, we will not do the override sections of the code

Thanks for the update of the function.

if the coordinate variables don't exist, we will not do the override sections of the code

I'm not sure that's quite true. The first time this function is called in the flow of the code is within hoss.dimension_utilities::get_prefetch_variables. The function gets called if there are no required dimensions. Then within this function you are looking at the coordinates metadata attribute on each required variable, and trying to retrieve information on each listed coordinate. If the metadata attribute is incorrect and points to non-existent variables, then at that point, the code is asking VarInfoFromDmr to do something it can't do (varinfo.get_variable(coordinate).is_longitude() or varinfo.get_variable(coordinate).is_latitude()), and it will raise an exception. Specifically, it will say that None does not have a method is_latitude or is_longitude, because varinfo.get_variable('something that doesn't exist') will return None.

I think this failure case needs to be handled, most likely by catching the exception and re-raising a more user-friendly one.

owenlittlejohns · 2024-10-02T21:14:27Z

hoss/dimension_utilities.py

+    return is_dimension_ascending(lat_col)
+
+
+def get_lat_lon_arrays(


You're right, sorry, the return value is a list of coordinate names, but the idea is still applicable. (The main looping logic is essentially duplicated between the two functions, see L168-L174 versus L278-L283: those are doing the same loops and the same checks)

That said, I think there's a bigger concern here for collections with multiple grids (and therefore multiple groups of variables in the coordinates references), such as SPL3FTP. In the function below the logic is going to go through all coordinates and check each one to see if it is a latitude or a longitude. You're getting the list of coordinates (via a few other functions) from get_coordinate_variables (originally called here), which would have multiple latitude/longitude references for SPL3FTP (if a user requests both polar and global variables). If you are trying to handle both the global and polar grid, you're going to need to sometimes get the global coordinates and sometimes the polar, but this function will always just get whichever are listed last (because the loop will get to them last and assign them to lat_arr and lon_arr, even if something is already assigned to those variables).

hoss/dimension_utilities.py

owenlittlejohns · 2024-10-02T21:29:52Z

hoss/dimension_utilities.py

+        prefetch_dataset, coordinates, varinfo
+    )
+
+    geo_grid_corners = get_geo_grid_corners(prefetch_dataset, coordinates, varinfo)


This still feels like an unnecessarily complicated methodology. You could just project the entire latitude and longitude arrays using pyproj and take one row and one column from the projected output. Is it to reduce the memory usage? SPL3FTP has arrays of (2, 406, 964) and (2, 500, 500) - those size arrays will not be a problem for memory usage.

The method we have here adds a bunch of functions and complexity that would otherwise not be needed.

will look into it

hoss/spatial.py

owenlittlejohns · 2024-10-02T22:17:52Z

hoss/spatial.py

+    """This function returns a dictionary containing the minimum and maximum
+    index ranges for a pair of lat/lon coordinates, e.g.:
+
+    index_ranges = {'/x': (20, 42), '/y': (31, 53)}


The initial bit of this description (the bit people are most likely to read) doesn't quite have enough information. I think the disconnect is due to referring to latitude and longitude and then the example having x and y.

Also - the names don't actually match what your function produces (currently "projected_x" and "projected_y" for all grids)

updated it - 802fe0e

But may need to revise it - if I pursue new names to support multiple grids

Okay. Thanks for the update. I'll close this comment thread, and we can continue to discuss the multiple-grid things in this thread. (That's still an issue that will have to be resolved in this PR)

owenlittlejohns · 2024-10-02T22:18:54Z

hoss/spatial.py

+    projected_x = 'projected_x'
+    projected_y = 'projected_y'


For granules with multiple grids, these names will clash for both grids.

I think you likely want some nice naming function (likely deriving the names from the coordinate variables used), and then that function can be used in add_index_range to determine which entries in the cache to use.

This is still a significant issue. If a user requests variables from both the SMAP L3 global and polar grids, this function will try to assign the x and y dimension index ranges for both to keys called projected_x and projected_y in the cache of index ranges.

hoss/spatial.py

…from_coordinates

owenlittlejohns · 2024-10-11T23:44:43Z

hoss/coordinate_utilities.py

+) -> bool:
+    """
+    returns the list of required variables without any
+    dimensions
+    """


Two quick things here:

The return type in the type hint is wrong: it should be set[str] not bool.

It would probably be clearer in the documentation string to say "set of required variables" not "list of required variables". (I'm guessing you meant list in a generic sense, rather than the Python type, but it's a little ambiguous)

owenlittlejohns · 2024-10-11T23:45:49Z

hoss/coordinate_utilities.py

+
+
+def get_variables_with_anonymous_dims(
+    varinfo: VarInfoFromDmr, required_variables: set[str]


Nitpick: It probably isn't necessary in this function to refer to the variables as required_variables. That is a piece of information outside of the scope of this function, maybe a better choice of name would be variable_names.

hoss/spatial.py

owenlittlejohns

Thanks for adding some unit tests - I'll keep an eye out for more in future commits.

I resolved older comments I had left, which now look to be taken care of (thanks for that). There are still outstanding older items, and I've added some more comments on things that I looked at in more detail this time around. (To be honest, given the huge scale of this PR, it's hard to review it all in a single go, and so there are still bits I'm spotting only now, despite trying to go through the PR a couple of times)

owenlittlejohns · 2024-10-12T00:31:16Z

hoss/coordinate_utilities.py

+    if lat_arr.ndim > 1:
+        col_size = lat_arr.shape[0]
+        row_size = lat_arr.shape[1]
+    if (lon_arr.shape[0] != lat_arr.shape[0]) or (lon_arr.shape[1] != lat_arr.shape[1]):
+        raise IrregularCoordinateDatasets(lon_arr.shape, lat_arr.shape)
+    if lat_arr.ndim and lon_arr.ndim == 1:
+        col_size = lat_arr.size
+        row_size = lon_arr.size


This is a bit wonky:

The middle check that the array sizes are equal explicitly checks the size of the 0th and 1st axes of the array. But after that you have a condition for whether the arrays only have one dimensions. This means one of two things:

Either the last bit with lat_array.ndim and lon_arr.ndim are both 1 will never get reached (because they are always 2-D or higher)

The coordinate arrays could be 1-D, and then the check for lon_arr.shape[1] != lat_arr.shape[1] will raise an exception, because the shape tuple doesn't have enough elements.

owenlittlejohns · 2024-10-12T00:33:45Z

hoss/coordinate_utilities.py

+        row_size = lat_arr.shape[1]
+    if (lon_arr.shape[0] != lat_arr.shape[0]) or (lon_arr.shape[1] != lat_arr.shape[1]):
+        raise IrregularCoordinateDatasets(lon_arr.shape, lat_arr.shape)
+    if lat_arr.ndim and lon_arr.ndim == 1:


This condition isn't doing what I believe you think it is.

My guess is you are trying to check if both lat_arr and lon_arr have 1 dimension. What's really happening is that you are checking if lat_arr.ndim has a "truthy" value (so isn't 0, where it's an integer), and then you are checking if lon_arr.ndim == 1. If it helps, the checks are really:

if (lat_arr.ndim) and (lon_arr.ndim == 1):

I think what you are trying to do is:

if lat_arr.ndim == 1 and lon_arr.ndim == 1:

owenlittlejohns · 2024-10-12T00:35:59Z

hoss/coordinate_utilities.py

+
+    """
+    override_variable = varinfo.get_variable(variable_name)
+    projected_dimension_name = ''


It would fit much better with the rest of the style of the code if you used else for these sorts of default values, instead of declaring them and then overriding them if a condition is met.

Yes, it's a personal preference, but it's consistent with the rest of the code (which is the more important thing here).

owenlittlejohns · 2024-10-12T00:39:27Z

hoss/coordinate_utilities.py

+    row_size = 0
+    col_size = 0


This is one example but there are quite a few places in this code that I think default values are being used instead of raising a genuine exception. If none of the conditions below are met, then it would be much better to catch the issue now and raise a user-friendly exception before later code tries to use these values and finds it can't do what it needs to (and raises some unintelligible exception to the end user).

I think the question to ask with a bunch of these functions is: if they fall back on the default values, can the rest of the code work using those return values. I think in this case (and a few others) the honest answer is no.

owenlittlejohns · 2024-10-12T00:43:47Z

hoss/coordinate_utilities.py

+    lat_col = lat_arr[:, 0]
+    lon_row = lon_arr[0, :]


This is making an assumption that latitude and longitude always represent the same axes in an array for all collections. We know that isn't true: for example the GPM_3IMERGHH collection from GES DISC have things the wrong way around (time, lon, lat).

It would be better to take both arrays and check one row and one column of each to find which array is varying along each dimension.

owenlittlejohns · 2024-10-12T01:50:54Z

tests/unit/test_spatial.py

+    def test_get_x_y_index_ranges_from_coordinates(
+        self,
+        mock_get_x_y_extents,
+        mock_get_dimension_index_range,
+    ):


This is a good test!

You definitely need either one more that requests SMAP L3 data for multiple grids, or this test could be updated to do so using a collection that has multiple grids.

owenlittlejohns · 2024-10-12T01:53:23Z

tests/unit/test_coordinate_utilities.py

+        # lat_arr = prefetch_dataset[self.latitude][:]
+        # lon_arr = prefetch_dataset[self.longitude][:]
+


These commented out lines should be removed.

owenlittlejohns · 2024-10-12T01:59:55Z

tests/unit/test_coordinate_utilities.py

+                lat_fill,
+                lon_fill,
+            )
+            for actual, expected in zip(actual_geo_corners, expected_geo_corners):


A couple of things here:

Why only self.assertAlmostEqual? You are defining the expected values, so you could define them to be exactly what you need.

(Possibly a personal preference) zip makes things a little more complicated. Instead you could do something like:

for index, expected_corner in enumerate(expected_geo_corners): self.assertTupleEqual(actual_geo_corners[index], expected_corner)

If you stick with zip, could you maybe tweak the variables in the loop so they are actual_corner and expected_corner. Just to make it easier when reading this test back. Thanks!

owenlittlejohns · 2024-10-12T02:03:00Z

tests/unit/test_coordinate_utilities.py

+
+        """
+
+        expected_result1 = np.array([0, 1, 2, 3, 4])


Something of a theme here - there are a bunch of variable names in these tests that are really vague (expected_result1, expected, expected_result, similar things with actual). It would be clearer to the reader if the names were more specific. Something like expected_valid_indices.

Unit tests are a secondary form of documentation for developers, and can be really informative. It's really helpful (to me at least 😅) if they are as easy to read as possible.

owenlittlejohns · 2024-10-12T02:05:05Z

tests/unit/test_coordinate_utilities.py

Thanks for starting to add in the tests! A recommendation for next time would be to couple the writing of a function with the accompanying tests, instead of leaving all the tests to the end. (I tend to write tests for functions just after writing the code for the function itself, because that way it's still fresh in my head, but also when I move on from that function, I feel confident that it works when I call it elsewhere)

sudha-murthy added 6 commits September 9, 2024 10:24

DAS-2232 Initial commit for SMAP L3 spatial subset changes

79b4c3f

corrections from pre-commit checks

789cc6b

DAS-2232 - updates to correct unit test failures

c4abf26

DAS-2232 updates for unit test failures

be280f0

DAS-2232 - all unit tests pass

2a48f93

DAS-2232 updates to version number

2c93a4b

sudha-murthy requested review from owenlittlejohns and lyonthefrog September 10, 2024 18:48

sudha-murthy commented Sep 10, 2024

View reviewed changes

hoss/spatial.py Show resolved Hide resolved

sudha-murthy commented Sep 10, 2024

View reviewed changes

hoss/dimension_utilities.py Show resolved Hide resolved

sudha-murthy requested a review from flamingbear September 10, 2024 19:01

DAS-2232 - updated notebook version due to Synk vulnerability. Also r…

dfb1e15

…emoved duplicate method

sudha-murthy marked this pull request as draft September 12, 2024 17:40

DAS-2232 fixed spatial subsetting bugs introduced when removing dupli…

e2ff61f

…cate nethods

sudha-murthy marked this pull request as ready for review September 13, 2024 05:09

autydp reviewed Sep 17, 2024

View reviewed changes

hoss/hoss_config.json Show resolved Hide resolved

autydp reviewed Sep 17, 2024

View reviewed changes

hoss/dimension_utilities.py Outdated Show resolved Hide resolved

flamingbear requested changes Sep 20, 2024

View reviewed changes

owenlittlejohns requested changes Sep 20, 2024

View reviewed changes

autydp reviewed Sep 23, 2024

View reviewed changes

hoss/dimension_utilities.py Outdated Show resolved Hide resolved

autydp reviewed Sep 23, 2024

View reviewed changes

hoss/dimension_utilities.py Show resolved Hide resolved

sudha-murthy added 4 commits September 26, 2024 03:04

DAS-2232 initial commit after PR feedback

756f7c0

DAS-2232 - fixed get_variable_crs and a few minor corrections

53f1660

DAS-2232 - comments updated for the get_spatial_index_ranges method

f9f5e8b

DAS-2232 simplified get_spatial_index_ranges method

f836ee6

flamingbear reviewed Sep 26, 2024

View reviewed changes

CHANGELOG.md Outdated Show resolved Hide resolved

flamingbear reviewed Sep 26, 2024

View reviewed changes

CHANGELOG.md Outdated Show resolved Hide resolved

flamingbear reviewed Sep 26, 2024

View reviewed changes

CHANGELOG.md Outdated Show resolved Hide resolved

flamingbear reviewed Sep 26, 2024

View reviewed changes

CHANGELOG.md Outdated Show resolved Hide resolved

sudha-murthy added 10 commits October 1, 2024 03:04

DAS-2232 - fixed assumption of top left origin

f07b544

DAS=2232 - some refactoring

3b453e5

DAS-2232 - updates to hoss_config.json description fields

681b20d

DAS-2232 - updated the names for required_dimensions to required_vari…

5d609c9

…ables

DAS-2232 - added exception if one of the coordinate datasets are missing

91c51c0

DAS-2232 - updated CHANGELOG.md

2296a35

DAS-2232 - small bug fix in get_override_projection_dimension_name

2efc4c7

DAS-2232 - updates to comments

f628166

DAS-2232 - added check for fillvalue

ebac2a0

DAS-2232 - comments and minor changes

3b6d605

owenlittlejohns reviewed Oct 2, 2024

View reviewed changes

hoss/spatial.py Outdated Show resolved Hide resolved

owenlittlejohns reviewed Oct 2, 2024

View reviewed changes

hoss/spatial.py Outdated Show resolved Hide resolved

owenlittlejohns reviewed Oct 3, 2024

View reviewed changes

hoss/spatial.py Outdated Show resolved Hide resolved

sudha-murthy added 12 commits October 3, 2024 11:18

DAS-2232 - simplified get_coordinate_variables based on PR feedback

802fe0e

DAS-2232 - added method to check for variables with no dimensions

60fb22a

DAS-2232 - added unittest for get_variable_crs

631dc24

DAS-2232 - added a module to contain coordinate methods

1e7bc35

DAS-2232 - moved new methods to a separate file

36e15c7

DAS-2232 - removed accidental checkin of an incomplete unit test

5c5eb85

DAS-2232 - added unit test for the new method - get_x_y_index_ranges_…

80c2fb2

…from_coordinates

DAS-2232 - added unit test for the new method - get_x_y_index_ranges_…

30eccd0

…from_coordinates

DAS-2232 - added some unit tests and some bug fixes

16872b7

DAS-2232 - added some unit tests and some bug fixes

822758f

DAS-2232 - minor initialization fix

7883465

DAS-2232 - added unit test for get_valid_indices

dd98e81

owenlittlejohns reviewed Oct 11, 2024

View reviewed changes

owenlittlejohns reviewed Oct 12, 2024

View reviewed changes

		@@ -55,6 +59,42 @@ def is_index_subset(message: Message) -> bool:
		)


		def get_override_projected_dimensions(

		return is_dimension_ascending(lat_col)


		def get_lat_lon_arrays(



		def get_variables_with_anonymous_dims(
		varinfo: VarInfoFromDmr, required_variables: set[str]

		# lat_arr = prefetch_dataset[self.latitude][:]
		# lon_arr = prefetch_dataset[self.longitude][:]

DAS-2232 - Functionality added to support SMAP L3 products #15

Are you sure you want to change the base?

DAS-2232 - Functionality added to support SMAP L3 products #15

Conversation

sudha-murthy commented Sep 10, 2024 • edited Loading

Description

Jira Issue ID

Local Test Steps

PR Acceptance Checklist

flamingbear left a comment

Choose a reason for hiding this comment

owenlittlejohns left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

owenlittlejohns Sep 20, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

owenlittlejohns commented Sep 24, 2024

owenlittlejohns left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

owenlittlejohns Oct 12, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sudha-murthy Oct 3, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

owenlittlejohns left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sudha-murthy commented Sep 10, 2024 •

edited

Loading

owenlittlejohns Sep 20, 2024 •

edited

Loading

owenlittlejohns Oct 12, 2024 •

edited

Loading

sudha-murthy Oct 3, 2024 •

edited

Loading