rsheftel / pandas_market_calendars

Exchange calendars to use with pandas for trading applications
MIT License
777 stars 172 forks source link

Inconsistent use of time zones #260

Open rdong8 opened 1 year ago

rdong8 commented 1 year ago

For instance, for the CME energy and metals class, we have no time zone supplied here:

https://github.com/rsheftel/pandas_market_calendars/blob/bc642ec83e19ec69a289743b7d7b30b7c6c17a4d/pandas_market_calendars/exchange_calendar_cme_globex_energy_and_metals.py#L101

But it is used here:

https://github.com/rsheftel/pandas_market_calendars/blob/bc642ec83e19ec69a289743b7d7b30b7c6c17a4d/pandas_market_calendars/exchange_calendar_cme_globex_energy_and_metals.py#L125

https://github.com/rsheftel/pandas_market_calendars/blob/bc642ec83e19ec69a289743b7d7b30b7c6c17a4d/pandas_market_calendars/exchange_calendar_cme_globex_energy_and_metals.py#L133

https://github.com/rsheftel/pandas_market_calendars/blob/bc642ec83e19ec69a289743b7d7b30b7c6c17a4d/pandas_market_calendars/exchange_calendar_cme_globex_energy_and_metals.py#L136

rsheftel commented 1 year ago

Agree that this should be cleaned up for visual consistency. As a practical matter there is a .tz method on every calendar that defines the default time zone. If you would like to submit a PR that would be great.

rdong8 commented 1 year ago

tz must be defined before we refer to it, so is it ok to move all the definitions of the tz property right above the definition of regular_market_times?

class CMEGlobexEquitiesExchangeCalendar(CMEGlobexBaseExchangeCalendar):

    aliases = ["CME Globex Equity"]

    @property
    def tz(self): return timezone("America/Chicago")

    regular_market_times = {
        "market_open": ((None, time(17), -1),), # offset by -1 day
        "market_close": ((None, time(16)),),
    }
rsheftel commented 1 year ago

Yes