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

Update to python 3.11 #181

Merged
merged 6 commits into from
Jul 28, 2023
Merged

Update to python 3.11 #181

merged 6 commits into from
Jul 28, 2023

Conversation

bdestombe
Copy link
Collaborator

netcdf4 to minimum of 1.6.4. Added flox as requirement to speed up resampling.

netcdf4 to minimum of 1.6.4. Added flox as requirement to speed up resampling.
@bdestombe
Copy link
Collaborator Author

#172

@BSchilperoort
Copy link
Collaborator

Added flox as requirement to speed up resampling

Nice, I have been using flox a lot recently. It has some really nice features (such as a groupby_bins that allows overlapping groups)

@bdestombe
Copy link
Collaborator Author

@BSchilperoort
Copy link
Collaborator

Seems like there is an issue with string type variables in our data. My opinion is that xarray should be catching that before dispatching the writing to the netcdf4 library.

Anyway, I would think that we can manually set the compression/encoding to None for any string variables that are in the data, which then fixes the issue?

@bdestombe
Copy link
Collaborator Author

Okay that worked. It is now stuck in to_mf_netcdf() and I can't seem to solve that one.

@BSchilperoort
Copy link
Collaborator

Okay that worked. It is now stuck in to_mf_netcdf() and I can't seem to solve that one.
It still goes wrong in the encoding step.

If you make this change in datastore.py the test passes:
image

@BSchilperoort
Copy link
Collaborator

Working on a patch, I'll push to this branch

@bdestombe
Copy link
Collaborator Author

bdestombe commented Jul 28, 2023

The problem was that on loaded dask arrays, strings are loaded as datatype objects and therefore were not identified as strings. As a result those datatype objects were also getting a zlib=True in their encodings.

With this fix the other data_vars continue to be compressed

@BSchilperoort
Copy link
Collaborator

Oh nice find! Dask arrays can be tricky that way

Copy link
Collaborator

@BSchilperoort BSchilperoort left a comment

Choose a reason for hiding this comment

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

Awesome! Good work finding out the dask type issue

CHANGELOG.rst Outdated
@@ -7,10 +7,13 @@ dev
New features

* Improved the functionality of `merge_double_ended`, by adding a check that handles measurements missing in one channel while present in the other ([#171](https://github.com/dtscalibration/python-dts-calibration/pull/171))
* Support for Python 3.11
* Requiring netcdf4 >= 1.6.4
Copy link
Collaborator

Choose a reason for hiding this comment

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

Not really a new feature I'd think?

I think I like such a layout more for changelogs:

Fixed:

  • Bug in abc

Added:

  • Support for Python 3.11

Changed:

  • Now requires netcdf >= 1.6.4

@bdestombe bdestombe merged commit 26b6665 into main Jul 28, 2023
@bdestombe bdestombe deleted the Support-python311 branch July 28, 2023 14:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants