search

pastas / pastastore

:spaghetti: :convenience_store: Tools for managing timeseries and Pastas models
https://pastastore.readthedocs.io
MIT License
15 stars 4 forks source link

[Enhancement] Force Data type of oseries index to strings #39

Closed raoulcollenteur closed 3 years ago

raoulcollenteur commented 3 years ago

Finally got to move some projects to Pastastore, works very well two far! Small issue here:

I have time series with numbers as series name. Although I force these numbers to be strings, this may still cause failure at a later stage (e.g., store.get_statistics(["evp"]).

It would be nicety simply force the index of store.oseries to strings, to prevent any issues.

dbrakenhoff commented 3 years ago

I saw you self-assigned this, but since I was in the middle of a big (internal) update, I figured I'd add the fix for this.

Both the _add_series and add_model methods now force name to be of type str. This won't fix old pastastores with non-str names, but for new ones this should take care of things?

dbrakenhoff commented 3 years ago

PS. can you let met know if this fixes it? I want to make a new release soon and it would be nice to include this fix.

raoulcollenteur commented 3 years ago

The fix unfortunately does not work for the models I am testing it on.

Seems to be going wrong in line : https://github.com/pastas/pastastore/blob/a1fba2bc51427432df59b04aed12c16d336ae304/pastastore/base.py#L786

I think this is because we don't enforce strings in Pastas itself right>? Maybe we should fix it there..

raoulcollenteur commented 3 years ago

just checked: name = str(mdict["oseries"]['name']) fixes it.

mdict is from a .pas file right? If so, I think we should open a new issue in pastas and fix it there.

dbrakenhoff commented 3 years ago

That makes sense, my change only fixed it on the pastastore side, not in the pastas model oseries. Can you commit the change (and maybe also for the stresses)?

dbrakenhoff commented 3 years ago

mdict is from a pas file yes, it is basically the result of pastas.Model.to_dict(series=False)

raoulcollenteur commented 3 years ago

Just committed the fix. However, I will open an issue in Pastas, to double check data types when reading models from .pas-files with ps.load_model.