BLD: Build shared c dependencies as a library
State: closed

- [ ] closes #30873 (Replace xxxx with the Github issue number)
- [ ] [Tests added and passed](https://pandas.pydata.org/pandas-docs/dev/development/contributing_codebase.html#writing-tests) if fixing a bug or adding a new feature
- [ ] All [code checks passed](https://pandas.pydata.org/pandas-docs/dev/development/contributing_codebase.html#pre-commit).
- [ ] Added an entry in the latest `doc/source/whatsnew/vX.X.X.rst` file if fixing a bug or adding a new feature.

Does anyone else want to test this? I can only test with -j4 on my old 2-core i3.

Labels:
name: Build
description: Library building on various platforms

Comments:
Does this affect build size or speed or inlining or ...?
I like this PR, and want to try it out, but my laptop where I do 8-core builds on Windows that always fails is out of commission for a week or so.  Once I get that back, I will give it a try and let you know the outcome.
@lithomas1 do you know what makes the compilation process different for these versus shared libraries created from compiled Cython modules? I'm slightly -1 on adding this complexity and certainly -1 for expanding it beyond these libraries for what seems like a very marginal gain


I just tried this PR to build the latest version on my Windows laptop using `python setup.py build_ext -j 8` and it worked perfectly.  I usually have to do it 2 or 3 times because of the conflicts.  So, as the person who created the original issue, I'm +8 (to match my cores) on this!

> @lithomas1 do you know what makes the compilation process different for these versus shared libraries created from compiled Cython modules? I'm slightly -1 on adding this complexity and certainly -1 for expanding it beyond these libraries for what seems like a very marginal gain

The problem is that there seems to be no way to link against other Extensions(the shared library made from the Cython module) from another Extension. The first option from [SO Link](https://stackoverflow.com/questions/57609741/link-against-another-python-c-extension#comment115546116_57609741) is what I'm doing (the other options don't seem to be too relevant, and are also going over my head). 

The other benefit of this is that the parser and the tslibs c sources don't get recompiled many times.
Would this potentially help solve https://github.com/pandas-dev/pandas/issues/47305?
> The problem is that there seems to be no way to link against other Extensions(the shared library made from the Cython module) from another Extension. The first option from [SO Link](https://stackoverflow.com/questions/57609741/link-against-another-python-c-extension#comment115546116_57609741) is what I'm doing (the other options don't seem to be too relevant, and are also going over my head).

Does the `depends` keyword argument not help here?
> Would this potentially help solve #47305?

the longer #47305 remains unresolved, I think the higher chance of failure when bisecting regressions (but can maybe alter the bisect process if this becomes an issue)

so +1 here if solves #47305
ok let's merge this and see
> > The problem is that there seems to be no way to link against other Extensions(the shared library made from the Cython module) from another Extension. The first option from [SO Link](https://stackoverflow.com/questions/57609741/link-against-another-python-c-extension#comment115546116_57609741) is what I'm doing (the other options don't seem to be too relevant, and are also going over my head).
> 
> Does the `depends` keyword argument not help here?

I'm pretty sure I tried this and it didn't work, let me double check.
I am fine with this for now if it fixes immediate issues. Can always clean up later. Also exploring different build tooling in #47380 which could help
> I just tried this PR to build the latest version on my Windows laptop using `python setup.py build_ext -j 8` and it worked perfectly. I usually have to do it 2 or 3 times because of the conflicts. So, as the person who created the original issue, I'm +8 (to match my cores) on this!

So I upgraded my Visual Studio installation to Visual Studio 2019, and tried this PR,  using `python setup.py build_ext -j 8` and now have the original problem that I reported in #30873  .  Sigh...

Note that using `python setup.py build_ext -j 4` works fine.

Having said that, I think I know what the issue is, which is that there were still references to `np_datetime.c` that appeared after a merge, so I did edits to the `setup.py` to fix that, and then it worked.   Created a PR https://github.com/lithomas1/pandas/pull/16 for @lithomas1 to review and integrate on his repo.  Then he can update his branch within this PR.

@lithomas1 see my suggested changes in https://github.com/lithomas1/pandas/pull/16
I haven't touched this PR in a while, since we are considering Meson(working on exploring this front)/Cmake as our new build system. If the other core devs still think this is worthwhile, I am happy to pick it back up though.
> I haven't touched this PR in a while, since we are considering Meson(working on exploring this front)/Cmake as our new build system. If the other core devs still think this is worthwhile, I am happy to pick it back up though.

@lithomas1 I'm happy if you are working on different approach instead.

xref https://github.com/pandas-dev/pandas/pull/47081#issuecomment-1157504061

I don't recall seeing `undefined symbol: pandas_datetime_to_datetimestruct` locally or on bisect workflows for a while. Not an issue on CI at the moment either, #47341


This pull request is stale because it has been open for thirty days with no activity. Please [update](https://pandas.pydata.org/pandas-docs/stable/development/contributing.html#updating-your-pull-request) and respond to this comment if you're still interested in working on this.
It might be time to revive this. This might fix the CI and I don't think the meson work will be ready before at least the end of this year. cc @WillAyd 
Hmm I _think_ I'd prefer not to hack setuptools like this versus just using Meson. Stuck with anything in particular on that I might be able to help with?


FWIW if you look at the build log this still generates the `np_datetime.o` file multiple times. Assuming a race condition between the various compilation / linking phases is the root of the issue I'm not sure this solves anything
> FWIW if you look at the build log this still generates the `np_datetime.o` file multiple times. Assuming a race condition between the various compilation / linking phases is the root of the issue I'm not sure this solves anything

See my message above:  https://github.com/pandas-dev/pandas/pull/47081#issuecomment-1157912923

I created a PR for @lithomas1 that fixes this issue.
Hmm interesting. It looks like the build log shows that file still getting generated a few times. 

Not opposed to merging this as is just don't think this does what we think it does.  Just want to be wary of continued setuptools hacks going forward 
> Hmm I _think_ I'd prefer not to hack setuptools like this versus just using Meson. Stuck with anything in particular on that I might be able to help with?

I working out a couple of bugs in meson and meson-python. mesonbuild/meson#10639 is the one I'm looking into right now.


> > FWIW if you look at the build log this still generates the `np_datetime.o` file multiple times. Assuming a race condition between the various compilation / linking phases is the root of the issue I'm not sure this solves anything
> 
> See my message above: [#47081 (comment)](https://github.com/pandas-dev/pandas/pull/47081#issuecomment-1157912923)
> 
> I created a PR for @lithomas1 that fixes this issue.

Merged it, thanks.



> Hmm interesting. It looks like the build log shows that file still getting generated a few times.
> 
> Not opposed to merging this as is just don't think this does what we think it does. Just want to be wary of continued setuptools hacks going forward

Yeah, this could end up not worth it ifit causes a performance egression somewhere or somehow borks the wheels.
Around 50% of the time I have to rebuild due to the undefined symbol issue with `-j 4`. I have a 16 core (32 threads) machine and with this PR running

```
find . -type f -exec touch {} +
python setup.py build_ext -j 32
```

three times all builds were successful. Doing the same on main, all three failed. Based on the conversation above, I think that makes me +32 on this PR.
Small update on this:

Thanks all for the encouragement on this, and sorry for the long period without updates. 
I think I planned to get this in in the meantime before I finished meson work, but it isn't worth it at this point to spend the time investigating perf, and other issues that this may cause.

Work on meson is a mixed bag

- I can now compile and test on CI on Windows, macOS, and Linux.
- Sadly, we have failing tests. :(
        - The only issue really worth worrying about is the SAS failures. Looks like there is a problem exporting the correct module name there for the Cython Extension Module.
        - I think some of our tests also depend on pandas being built inplace so which is causing some of the failures.
- Finally, I have to figure out how to generate the pandas version from the tag. This will fix most of the other failures.

The bad news is I still don't think we'll be able to get meson enabled as the default build system before the end of the year. I've been upstreaming fixes to both the meson and meson-python libraries, and probably is going to be a while before they cut another release. 

The good news is that we'll probably have some sort of meson support in main by next month(hopefully?) for brave people to try, it just won't be in the CI until meson and meson-python cut a new release with my fixes in them. 
> Finally, I have to figure out how to generate the pandas version from the tag. This will fix most of the other failures.

This is managed today via versioneer, which I think is only supported via setuptools. Might need to look at upstreaming support for meson in that as well
Re the version, it depends on what you want:
1. Have an internal version string which contains the commit hash, but a project version (ending up in the wheel filename) of `1.6.0.dev0` without git hash.
2. Also have the git hash reflected in the wheel version. This is what a number of projects using `versioneer` do.
3. Getting the project version directly from a git tag, ala `setuptools_scm`.

(2) isn't possible easily in Meson right now, because using its `vcs_tag` feature in the project version would trigger a full reconfigure -see https://github.com/mesonbuild/meson/issues/688#issuecomment-422757933. I'm not sure it would be accepted, although there is recent movement on the issue I linked (including a workaround implementation).

(3) seems to be more feasible.

SciPy used to do (2), and I switched it to (1) during the move to Meson. That has been fine so far. The benefit of having the git hash in the version is relatively minor, and it's still available at install time as `python -c "import scipy; print(scipy.__version__)"`.
> SciPy used to do (2), and I switched it to (1) during the move to Meson. That has been fine so far. The benefit of having the git hash in the version is relatively minor, and it's still available at install time as `python -c "import scipy; print(scipy.__version__)"`.

I'm a little worried that this will overwrite nightly wheels, though, since the filenames will be the same. This could be a problem if someone downstream is pinning the tests of pandas nightlies.

Is there something we can do in meson-python to workaround this? Since the version in meson has to be hard-coded, it might make sense for meson-python to take an option in the pyproject.toml, to get the version from git?
> Is there something we can do in meson-python to workaround this? Since the version in meson has to be hard-coded, it might make sense for meson-python to take an option in the pyproject.toml, to get the version from git?

That sounds like a reasonable thing to do in `meson-python`. I think all that is needed is to (a) actually retrieve the git-based version, and (b) use that in the wheel name and in the `METADATA` file. That said:
- this stuff is quite fragile (`versioneer` seems to be a never-ending source of bugs, it's way over-complicated), and
- different projects may want to make different choices regarding what the version numbering should be (e.g., just the git hash vs. an incrementing count since the last tag - the latter can be useful for nightlies but problematic with shallow clones). EDIT: also `setuptools_scm` like behaviour

So perhaps rather than baking it into `meson-python` itself, there could be some hook mechanism so projects can provide their own function which returns the desired version string. WDYT?
Will have to excuse any ignorance in the question but is it possible to continue to use setuptools with meson? 
It's perfectly fine to have two build systems live side-by-side in this repo. SciPy has had both `setuptools` and `meson` since last Christmas. The one choice to make is which of the two build systems gets invoked for `pip install .` - there's a single hook in `pyproject.toml`, so you cannot make that configurable.