search

cedadev / nappy

NASA Ames Processing in PYthon (NAPPy) - a Python library for reading, writing and converting NASA Ames files.
BSD 3-Clause "New" or "Revised" License
9 stars 13 forks source link

Release v2.0.2 #59

Closed agstephens closed 2 years ago

agstephens commented 2 years ago

@FObersteiner @LukeJones123, I have released v2.0.2. Please can you try it out and let me know if there are any problems. Thanks

FObersteiner commented 2 years ago

Works fine for me so far. The new var & units parsing capabilities are very helpful.

One thing that came up for me was that it might be useful (for me at least ^^) to dump VNAMEs when converting na to nc (i.e. to a global attribute maybe). The VNAMEs I have to deal with are not exactly standard and I want to provide sufficient info on those variables in the netCDF file as well (so the user doesn't have to open the na file additionally).

agstephens commented 2 years ago

Hi @FObersteiner, I am happy for us to add extra mappings between NC and NA formats as is useful. Do you want to propose a global attribute such as vnames_from_source_data ?

FObersteiner commented 2 years ago

Do you want to propose a global attribute such as vnames_from_source_data ?

yes, that sounds reasonable.

Another option might be to set the original VNAME from the na file as an attribute of the xr.DataArray (make a "vname_from_source_data" tag).

ReimarBauer commented 2 years ago

Hi @agstephens we had to change the build recipe. Not sure what the cause is. Just connecting our changes. https://github.com/conda-forge/nappy-feedstock/pull/9

Perhaps you can seperate the version information in a differen module? I guess that solves the import all itselfs dependency on built time. https://github.com/cedadev/nappy/blob/main/nappy/__init__.py

BastianZim commented 2 years ago

Just to add from conda-forge: The test folder also gets installed as a separate package according to our tools which can quickly pollute a user environment. It would be good to exclude that folder and not install it.

agstephens commented 2 years ago

Hi @BastianZim, In the current version, we have the tests directory outside of the main nappy package. Should we be doing anything else to exlude the tests.

BastianZim commented 2 years ago

Hi @agstephens (Note, this is about v2.0.2). It is unfortunately still included as a separate module. The problem with that is that when you install it using pip, a separate module called test is installed in your environment. If you then install another package that has the same problem, you get problems as suddenly two test packages are available. Ideally, the test folder should be excluded when installing with pip.

I have created https://github.com/cedadev/nappy/pull/60 but didn't add any author info since it's only a minor change. let me know if I should.

agstephens commented 2 years ago

Thanks @BastianZim , much appreciated. I have merged your PR.

agstephens commented 2 years ago

The outstanding part of this issue is now caught in #61, so closing this.