Skip to content

Conversation

niatimary
Copy link
Collaborator

  • Adds Series.drop_na() method to remove NaN values while preserving temporal order, complementing the existing Series.clean() method which removes NaNs and sorts data.
  • Add TestUISeriesClean.test_drop_na() in pyleoclim/tests/test_core_Series.py

Copy link
Collaborator

@CommonClimate CommonClimate left a comment

Choose a reason for hiding this comment

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

Please:

  • rename to dropna()
  • reuse tsbase.dropna() (see init_ for an example of how this is one)
  • this seems overkill:
time = ts_clean.__dict__["time"]
 value = ts_clean.__dict__["value"]

you should be able to access them by ts_clean.time and ts_clean.value. AI coding is not always best...

Copy link
Collaborator

@CommonClimate CommonClimate left a comment

Choose a reason for hiding this comment

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

You can simply re-use this bit:

 if dropna == True:
            value, time = tsbase.dropna(value, time, verbose=verbose)
            if keep_log == True:
                if len(self.log) > 0:
                    if self.log[0][0] == 'dropna' and self.log[0]['applied'] == True:
                        pass # no need to clog the log with redundant information
                    elif self.log[0][0] == 'clean_ts' and self.log[0]['applied']==True:
                        self.log[0]['legacy']=True
                        self.log += ({len(self.log): 'dropna', 'applied': dropna, 'verbose': verbose},)
                else:
                    self.log += ({len(self.log): 'dropna', 'applied': dropna, 'verbose': verbose},)
        elif dropna == False:
            pass
        else:
            raise ValueError('dropna should be a boolean')

@niatimary niatimary closed this Aug 26, 2025
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