robcarver17 / pysystemtrade

Systematic Trading in python
GNU General Public License v3.0
2.49k stars 798 forks source link

repocsv_spotfx_prices requires child class of FxPricesData #1376

Closed craigmediaservices closed 1 month ago

craigmediaservices commented 2 months ago

Issue:

Encountered a NotImplementedError(USE_CHILD_CLASS_ERROR) when attempting to initialize and populate the FX prices database using the add_fx_prices method from the fxPricesData class. This error was due to the direct instantiation of fxPricesData, an abstract base class, which lacks implementations for required methods like _add_fx_prices_without_checking_for_existing_entry.

Resolution: To resolve this, we switched to using parquetFxPricesData, a subclass of fxPricesData that properly implements all abstract methods required for handling FX prices stored in Parquet format. However, parquetFxPricesData requires a ParquetAccess instance initialized with a parquet_store_path, which can be dynamically retrieved from the project's configuration settings via dataBlob.

Testing:

Verified that the script correctly initializes and populates the FX prices without encountering NotImplementedError.
bug-or-feature commented 2 months ago

@craigmediaservices This is all wrong I'm afraid, it won't be merged. Data abstraction in PST should allow the importer to work whether you are set up to use Parquet or Arctic (or anything else). But somehow the FX importer was missed in the parquet refactor. If you want to see how it should have been done, see sysinit.futures.repocsv_multiple_prices for importing multiple prices. But in the FX importer use an instance of dataCurrency instead of diagPrices

craigmediaservices commented 1 month ago

That makes more sense. The bypass I used worked, but broke the data abstraction intention. I was digging around but couldn't figure out the needed subclass.