Skip to content

BUG: Period.now not taking kwargs #53375

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 6 commits into from
Jul 14, 2023
Merged

Conversation

lithomas1
Copy link
Member

Confirmed working with both setuptools/meson.

@lithomas1 lithomas1 added Bug Build Library building on various platforms Period Period data type labels May 24, 2023
@lithomas1 lithomas1 added this to the 2.1 milestone May 24, 2023
@mroeschke
Copy link
Member

Not sure if this build related error is due to this PR

   /home/runner/micromamba/envs/test/lib/python3.10/site-packages/Cython/Compiler/Main.py:370: FutureWarning: Cython directive 'language_level' not set, using '3str' for now (Py3). This has changed from earlier releases! File: /home/runner/work/pandas/pandas/pandas/_libs/tslibs/offsets.pxd
    tree = Parsing.p_module(s, pxd, full_module_name)

  Error compiling Cython file:
  ------------------------------------------------------------
  ...

      def get_rule_code_suffix(self) -> str:
          prefix = self._get_suffix_prefix()
          month = MONTH_ALIASES[self.startingMonth]
          weekday = int_to_weekday[self.weekday]
          return f"{prefix}-{month}-{weekday}"
                 ^
  ------------------------------------------------------------

  /home/runner/work/pandas/pandas/pandas/_libs/tslibs/offsets.pyx:3324:15: Cannot convert Unicode string to 'str' implicitly. This is not portable and requires explicit encoding.
  [35/164] Compiling C object pandas/_libs/tslibs/offsets.cpython-310-x86_64-linux-gnu.so.p/src_datetime_np_datetime.c.o
  [36/164] Compiling C object pandas/_libs/tslibs/np_datetime.cpython-310-x86_64-linux-gnu.so.p/meson-generated_pandas__libs_tslibs_np_datetime.pyx.c.o

@lithomas1 lithomas1 marked this pull request as draft June 13, 2023 15:09
@lithomas1
Copy link
Member Author

Marking as draft pending resolution of a Cython bug I think.
cython/cython#5484

@WillAyd
Copy link
Member

WillAyd commented Jun 13, 2023

Does this change impact all of the Cython functions? From the docs makes it seems like it could be a performance hit for cdef functions we call in tight loops

@lithomas1
Copy link
Member Author

Only affects single-arg functions.
(see cython/cython#3090).

The affected functions are def functions I think as well. I don't think we really do any loops in Python so perf impact shouldn't be too horrible (the Cython functions should use the cdef variants).

@lithomas1
Copy link
Member Author

Cython cut 3.0rc1 a little while ago, so this should be ready in a couple of days once the Github cache catches up.

@lithomas1 lithomas1 marked this pull request as ready for review July 14, 2023 15:36
@mroeschke mroeschke merged commit 747afca into pandas-dev:main Jul 14, 2023
@mroeschke
Copy link
Member

Thanks @lithomas1

@lithomas1 lithomas1 deleted the period-kwargs branch July 14, 2023 18:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Build Library building on various platforms Period Period data type
Projects
None yet
Development

Successfully merging this pull request may close these issues.

BUG: Period.now() does not accept kwarg "freq" but doc and typing say otherwise
3 participants