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

Features/various cleanup #186

Merged
merged 12 commits into from
Sep 29, 2023
Merged

Features/various cleanup #186

merged 12 commits into from
Sep 29, 2023

Conversation

ladsmund
Copy link
Contributor

@ladsmund ladsmund commented Sep 22, 2023

I have made a selection of minor changes and cleaning in the code.

I am focussing on moving data functionality for data level transformation to the functions toL1, toL2 and toL3 instead of having some mutation in AWS. Let me know in the discussion if this is not a good idea.

The responsibilities of AWS are generally unclear for me and I think we should have a discussion about the structure and direction of this repository.

Here are some header describing this PR:

  1. Cleaning up white spaces
  2. User python logging module instead of print for better log and print management
  3. Minor bug fixes like missing Exception instance on raise
  4. Moved clipValues/clip_values to level transformation functions toL1 and toL2 instead of AWS.

@ladsmund ladsmund force-pushed the features/various_cleanup branch from 4c68745 to e4487a7 Compare September 25, 2023 11:42
@ladsmund ladsmund force-pushed the features/various_cleanup branch from e4487a7 to ae572fe Compare September 25, 2023 12:23
The columns "msg_lat" and "mgs_lon" where explicitly added in AWS.__init__.
L0toL1, L1toL2 and L2toL3 functions are imported using absolute module paths as installed by the setup.py.
* Use separate module to avoid circular imports when used in other modules.
* Renamed according to PEP8 https://peps.python.org/pep-0008/#function-and-variable-names
There is a slight change in the behaviour because the clipping and variable filtering happens on each individual L1 dataset before the merge. This will not have any impacts on the output and makes MergeVars superfluous.
@ladsmund ladsmund force-pushed the features/various_cleanup branch from ae572fe to 684f2bf Compare September 25, 2023 12:24
@ladsmund ladsmund requested a review from PennyHow September 25, 2023 13:07
Copy link
Member

@PennyHow PennyHow left a comment

Choose a reason for hiding this comment

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

Generally looks great, just need that stray optional variable changing.

Once this is merged then I would like to do some additional clean-up. I see some functions do not follow conventional naming and like I said, I would like to move functions out of aws.py.

bin/getL0tx Show resolved Hide resolved
src/pypromice/process/L0toL1.py Show resolved Hide resolved
src/pypromice/process/aws.py Show resolved Hide resolved
@ladsmund ladsmund merged commit ed33cee into main Sep 29, 2023
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants