Skip to content

Conversation

@misi9170
Copy link
Collaborator

WindRose and WindTIRose have capabilities of reading data from CSVs using their own (static) read_csv_long() methods. However, the TimeSeries class, until now, has not supported such functionality. This PR adds a new static method read_csv() to the TimeSeries class to enable such functionality.

To be discussed:

  • The new TimeSeries.read_csv() allows users to specify a value_col, whereas WindRose.read_csv_long() and WindTIRose.read_csv_long() do not support reading in a value_col. Do we want to add this optional keyword arguments to WindRose.read_csv_long() and WindTIRose.read_csv_long()?
  • I chose to use TimeSeries.read_csv() rather than TimeSeries.read_csv_long() because I think the "long" format is implied by the "series" nature of the class. However, we could use read_csv_long instead to match the other wind data classes.
  • None of the WindData classes have a to_csv() method, but this also seems like a natural counterpart. We could consider adding one for TimeSeries and possibly also WindRose and WindTIRose

Tagging @paulf81 , @ejsimley in case they have thoughts on this.

@misi9170 misi9170 added the enhancement An improvement of an existing feature label Dec 19, 2025
@paulf81
Copy link
Collaborator

paulf81 commented Dec 22, 2025

  • The new TimeSeries.read_csv() allows users to specify a value_col, whereas WindRose.read_csv_long() and WindTIRose.read_csv_long() do not support reading in a value_col. Do we want to add this optional keyword arguments to WindRose.read_csv_long() and WindTIRose.read_csv_long()?

Sure, does seem reasonable!

  • I chose to use TimeSeries.read_csv() rather than TimeSeries.read_csv_long() because I think the "long" format is implied by the "series" nature of the class. However, we could use read_csv_long instead to match the other wind data classes.

TimeSeries.read_csv() does make sense would always be long. Could alias TimeSeries.read_csv_long() for completeness? Or maybe the more perfect is the function is defined in TimeSeries.read_csv_long() and TimeSeries.read_csv() is its alias?

  • None of the WindData classes have a to_csv() method, but this also seems like a natural counterpart. We could consider adding one for TimeSeries and possibly also WindRose and WindTIRose

I could see it. In FLASC I do sometimes call up FLORIS just for the WindRose classes and a work flow of loading a TimeSeries object, converting to WindRose and then exporting the WindRose to csv could have a place.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement An improvement of an existing feature

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants