Skip to content
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

release: Release 0.7.0 #308

Merged
merged 80 commits into from
Jul 23, 2023
Merged

release: Release 0.7.0 #308

merged 80 commits into from
Jul 23, 2023

Conversation

eitsupi
Copy link
Collaborator

@eitsupi eitsupi commented Jul 4, 2023

No description provided.

@eitsupi
Copy link
Collaborator Author

eitsupi commented Jul 4, 2023

(The checks fail because of the snapshot test including the version number. As a side note, the snapshot tests via testthat are skipped on CRAN by default because it is not executed if not NOT_CRAN="ture".)

@sorhawell
Copy link
Collaborator

It appears there is a hard limit of 20 minutes on compiling the package on CRAN
https://builder.r-hub.io/status/original/polars_0.7.0.tar.gz-8ec5459bde494544a183099bf6451494

@sorhawell
Copy link
Collaborator

NOTE update with ctb's in DESCRIPTION as of #307

@eddelbuettel
Copy link
Contributor

eddelbuettel commented Jul 5, 2023

It appears there is a hard limit of 20 minutes on compiling the package on CRAN

No that is just r-hub. You can't infer a limit at CRAN from r-hub -- but for CRAN most of what we know is in the CRAN Repository Policy page. Some larger packages (duckdb, arrow, ...) merrily take a longish time and I don't think I recall an explicit cap at CRAN.

@sorhawell
Copy link
Collaborator

sorhawell commented Jul 5, 2023

@etiennebacher @eitsupi
I have been struggling with the ExprDT docs. rextendr::document() or devtools::check(...) reworks all ExprDT methods and add usage section. R CMD check will fail that. make build and devtools::document() removes the usage sections. I have not figures out what makes the ExprDT docs different than other subnamespace or why different entries to roxygen produces different docs.

@eitsupi
Copy link
Collaborator Author

eitsupi commented Jul 5, 2023

I have been struggling with the ExprDT docs. rextendr::document() or devtools::check(...) reworks all ExprDT methods and add usage section. R CMD check will fail that. make build and devtools::document() removes the usage sections.

I have tried all except devtools::check() and they all seem to work the same. (Note that rextendr::document calls devtools::document)
There may be some other cause.

@sorhawell
Copy link
Collaborator

sorhawell commented Jul 15, 2023

I have some concerns about CRAN and my personal future enthusiasm:

  • I'm quite puzzled CRAN keep there own checking code secret and the submitter just have to manually bang against wall. Any future polars update could violate something, and we would have no idea from the GHA-check. It could take several workdays of frustration monthly to update.
  • We may finally ship polars without method documentation, because CRAN will refuse to recognize the methods as public, if not literally exported as some kind of pure functions.
  • We ship to CRAN without simd optimizations which is a potential significant performance drop. I think that could taint the upstream polars reputation. (edit: I was too dramatic here, sry :) )
  • I fear CRAN anyday will change their rust compilation rules and gives us few weeks notice to entirely rework the build. E.g. upstream rust-polars does also depend on github served crates, which could be outlawed soon.

If hitting too many obstacles would it make sense to not think of r-polars as CRAN first. But rather have a script that packages polars also for CRAN in the "downgraded version". Then we could just drop any doc page asked for, hardcode the packaged MAKEVARS to drop SIMD feature etc. and invite users in a message to install the "full featured better version" in 20 secs and use the awesome online documentation.

@sorhawell
Copy link
Collaborator

@sorhawell If submitting to CRAN is burdensome for you, I think we can either give up submitting this package to CRAN (in which case we can delete all the sections to adapt it to CRAN) or someone else, including myself, can do the submission to CRAN.

@eitsupi I think you would be a better CRAN/polars maintainer then I. I happily swap roles and/or follow your lead in these matters.

@eitsupi
Copy link
Collaborator Author

eitsupi commented Jul 15, 2023

We ship to CRAN without simd optimizations which is a potential significant performance drop. I think that could taint the upstream polars reputation.

I disagree, the default of Polars is to disable simd.
https://github.com/pola-rs/polars/blob/cc0795f7b980da385268d77216a9ae36dccdfcf4/polars/Cargo.toml#L17-L28

@sorhawell
Copy link
Collaborator

I disagree, the default of Polars is to disable simd. https://github.com/pola-rs/polars/blob/cc0795f7b980da385268d77216a9ae36dccdfcf4/polars/Cargo.toml#L17-L28

Sorry I mean py-polars ship nightly with simd via pip.

sorenwelling@Srens-MacBook-Pro rust % pip install polars --force
Collecting polars
  Downloading polars-0.18.7-cp38-abi3-macosx_10_7_x86_64.whl (18.5 MB)
     ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ 18.5/18.5 MB 6.8 MB/s eta 0:00:00
Installing collected packages: polars
  Attempting uninstall: polars
    Found existing installation: polars 0.18.4
    Uninstalling polars-0.18.4:
      Successfully uninstalled polars-0.18.4
Successfully installed polars-0.18.7
sorenwelling@Srens-MacBook-Pro rust % python
Python 3.10.9 (main, Dec 15 2022, 18:18:30) [Clang 14.0.0 (clang-1400.0.29.202)] on darwin
Type "help", "copyright", "credits" or "license" for more information.
>>> import polars as pl
>>> pl.build_info()
{'build': {'rustc': '/Users/runner/.rustup/toolchains/nightly-2023-06-23-x86_64-apple-darwin/bin/rustc', 'rustc-version': 'rustc 1.72.0-nightly (04075b320 2023-06-22)', 'opt-level': '3', 'debug': False, 'jobs': 3}, 'info-time': datetime.datetime(2023, 7, 12, 19, 12, 30, tzinfo=datetime.timezone.utc), 'dependencies': {'adler': '1.0.2', 'adler32': '1.2.0', 'ahash': '0.8.3', 'aho-corasick': '1.0.1', 'alloc-no-stdlib': '2.0.4', 'alloc-stdlib': '0.2.2', 'allocator-api2': '0.2.15', 'android-tzdata': '0.1.1', 'android_system_properties': '0.1.5', 'argminmax': '0.6.1', 'array-init-cursor': '0.2.0', 'arrow-format': '0.8.1', 'arrow2': '0.17.1', 'async-stream': '0.3.5', 'async-stream-impl': '0.3.5', 'async-trait': '0.1.68', 'atoi': '2.0.0', 'autocfg': '1.1.0', 'avro-schema': '0.3.0', 'base64': '0.21.2', 'bitflags': '1.3.2', 'brotli': '3.3.4', 'brotli-decompressor': '2.3.4', 'built': '0.6.0', 'bumpalo': '3.13.0', 'bytemuck': '1.13.1', 'bytemuck_derive': '1.4.1', 'bytes': '1.4.0', 'cargo-lock': '8.0.3', 'cc': '1.0.79', 'cfg-if': '1.0.0', 'chrono': '0.4.26', 'chrono-tz': '0.8.2', 'chrono-tz-build': '0.1.0', 'ciborium': '0.2.1', 'ciborium-io': '0.2.1', 'ciborium-ll': '0.2.1', 'comfy-table': '7.0.1', 'core-foundation-sys': '0.8.4', 'crc': '2.1.0', 'crc-catalog': '1.1.1', 'crc32fast': '1.3.2', 'crossbeam-channel': '0.5.8', 'crossbeam-deque': '0.8.3', 'crossbeam-epoch': '0.9.14', 'crossbeam-queue': '0.3.8', 'crossbeam-utils': '0.8.15', 'crossterm': '0.26.1', 'crossterm_winapi': '0.9.0', 'dyn-clone': '1.0.11', 'either': '1.8.1', 'enum_dispatch': '0.3.11', 'equivalent': '1.0.0', 'ethnum': '1.3.2', 'fallible-streaming-iterator': '0.1.9', 'fast-float': '0.2.0', 'flate2': '1.0.26', 'float-cmp': '0.9.0', 'foreign_vec': '0.1.0', 'form_urlencoded': '1.1.0', 'futures': '0.3.28', 'futures-channel': '0.3.28', 'futures-core': '0.3.28', 'futures-executor': '0.3.28', 'futures-io': '0.3.28', 'futures-macro': '0.3.28', 'futures-sink': '0.3.28', 'futures-task': '0.3.28', 'futures-util': '0.3.28', 'getrandom': '0.2.9', 'ghost': '0.1.9', 'git2': '0.16.1', 'glob': '0.3.1', 'half': '1.8.2', 'halfbrown': '0.2.2', 'hash_hasher': '2.0.3', 'hashbrown': '0.14.0', 'heck': '0.4.1', 'hermit-abi': '0.2.6', 'hex': '0.4.3', 'home': '0.5.5', 'iana-time-zone': '0.1.56', 'iana-time-zone-haiku': '0.1.2', 'idna': '0.3.0', 'indexmap': '2.0.0', 'indoc': '1.0.9', 'instant': '0.1.12', 'inventory': '0.3.6', 'itoa': '1.0.6', 'itoap': '1.0.1', 'jemalloc-sys': '0.5.3+5.3.0-patched', 'jemallocator': '0.5.0', 'jobserver': '0.1.26', 'js-sys': '0.3.63', 'jsonpath_lib': '0.3.0', 'lexical': '6.1.1', 'lexical-core': '0.8.5', 'lexical-parse-float': '0.8.5', 'lexical-parse-integer': '0.8.6', 'lexical-util': '0.8.5', 'lexical-write-float': '0.8.5', 'lexical-write-integer': '0.8.5', 'libc': '0.2.144', 'libflate': '1.4.0', 'libflate_lz77': '1.2.0', 'libgit2-sys': '0.14.2+1.5.1', 'libm': '0.2.7', 'libmimalloc-sys': '0.1.33', 'libz-sys': '1.1.9', 'lock_api': '0.4.9', 'log': '0.4.18', 'lz4': '1.24.0', 'lz4-sys': '1.9.4', 'matrixmultiply': '0.3.7', 'memchr': '2.5.0', 'memmap2': '0.5.10', 'memoffset': '0.9.0', 'mimalloc': '0.1.37', 'miniz_oxide': '0.7.1', 'mio': '0.8.8', 'multiversion': '0.7.2', 'multiversion-macros': '0.7.2', 'ndarray': '0.15.6', 'now': '0.1.3', 'ntapi': '0.4.1', 'num-complex': '0.4.3', 'num-integer': '0.1.45', 'num-traits': '0.2.15', 'num_cpus': '1.15.0', 'numpy': '0.19.0', 'once_cell': '1.17.2', 'parking_lot': '0.12.1', 'parking_lot_core': '0.9.7', 'parquet-format-safe': '0.2.4', 'parquet2': '0.17.2', 'parse-zoneinfo': '0.3.0', 'percent-encoding': '2.2.0', 'phf': '0.11.1', 'phf_codegen': '0.11.1', 'phf_generator': '0.11.1', 'phf_shared': '0.11.1', 'pin-project-lite': '0.2.9', 'pin-utils': '0.1.0', 'pkg-config': '0.3.27', 'planus': '0.3.1', 'polars': '0.30.0', 'polars-algo': '0.30.0', 'polars-arrow': '0.30.0', 'polars-core': '0.30.0', 'polars-error': '0.30.0', 'polars-io': '0.30.0', 'polars-json': '0.30.0', 'polars-lazy': '0.30.0', 'polars-ops': '0.30.0', 'polars-pipe': '0.30.0', 'polars-plan': '0.30.0', 'polars-row': '0.30.0', 'polars-sql': '0.30.0', 'polars-time': '0.30.0', 'polars-utils': '0.30.0', 'ppv-lite86': '0.2.17', 'proc-macro2': '1.0.59', 'py-polars': '0.18.7', 'pyo3': '0.19.0', 'pyo3-build-config': '0.19.0', 'pyo3-built': '0.4.7', 'pyo3-ffi': '0.19.0', 'pyo3-macros': '0.19.0', 'pyo3-macros-backend': '0.19.0', 'quote': '1.0.28', 'rand': '0.8.5', 'rand_chacha': '0.3.1', 'rand_core': '0.6.4', 'rand_distr': '0.4.3', 'rawpointer': '0.2.1', 'rayon': '1.7.0', 'rayon-core': '1.11.0', 'redox_syscall': '0.2.16', 'regex': '1.8.3', 'regex-syntax': '0.7.2', 'rle-decode-fast': '1.0.3', 'rustc-hash': '1.1.0', 'rustc_version': '0.4.0', 'rustversion': '1.0.12', 'ryu': '1.0.13', 'scopeguard': '1.1.0', 'semver': '1.0.17', 'seq-macro': '0.3.3', 'serde': '1.0.163', 'serde_derive': '1.0.163', 'serde_json': '1.0.96', 'signal-hook': '0.3.15', 'signal-hook-mio': '0.2.3', 'signal-hook-registry': '1.4.1', 'simd-json': '0.10.3', 'simdutf8': '0.1.4', 'siphasher': '0.3.10', 'slab': '0.4.8', 'smallvec': '1.10.0', 'smartstring': '1.0.1', 'snap': '1.1.0', 'socket2': '0.4.9', 'sqlparser': '0.34.0', 'static_assertions': '1.1.0', 'streaming-decompression': '0.1.2', 'streaming-iterator': '0.1.9', 'strength_reduce': '0.2.4', 'strum': '0.24.1', 'strum_macros': '0.25.0', 'syn': '2.0.18', 'sysinfo': '0.29.0', 'target-features': '0.1.4', 'target-lexicon': '0.12.7', 'thiserror': '1.0.40', 'thiserror-impl': '1.0.40', 'time': '0.1.45', 'tinyvec': '1.6.0', 'tinyvec_macros': '0.1.1', 'tokio': '1.28.2', 'toml': '0.5.11', 'unicode-bidi': '0.3.13', 'unicode-ident': '1.0.9', 'unicode-normalization': '0.1.22', 'unicode-width': '0.1.10', 'unindent': '0.1.11', 'url': '2.3.1', 'value-trait': '0.6.1', 'vcpkg': '0.2.15', 'version_check': '0.9.4', 'wasi': '0.11.0+wasi-snapshot-preview1', 'wasm-bindgen': '0.2.86', 'wasm-bindgen-backend': '0.2.86', 'wasm-bindgen-futures': '0.4.36', 'wasm-bindgen-macro': '0.2.86', 'wasm-bindgen-macro-support': '0.2.86', 'wasm-bindgen-shared': '0.2.86', 'wasm-timer': '0.2.5', 'web-sys': '0.3.63', 'winapi': '0.3.9', 'winapi-i686-pc-windows-gnu': '0.4.0', 'winapi-x86_64-pc-windows-gnu': '0.4.0', 'windows': '0.48.0', 'windows-sys': '0.48.0', 'windows-targets': '0.48.0', 'windows_aarch64_gnullvm': '0.48.0', 'windows_aarch64_msvc': '0.48.0', 'windows_i686_gnu': '0.48.0', 'windows_i686_msvc': '0.48.0', 'windows_x86_64_gnu': '0.48.0', 'windows_x86_64_gnullvm': '0.48.0', 'windows_x86_64_msvc': '0.48.0', 'xxhash-rust': '0.8.6', 'zstd': '0.12.3+zstd.1.5.2', 'zstd-safe': '6.0.5+zstd.1.5.4', 'zstd-sys': '2.0.8+zstd.1.5.5'}, 'features': ['ALL', 'ASOF_JOIN', 'AVRO', 'BINARY_ENCODING', 'BUILD_INFO', 'CROSS_JOIN', 'CSE', 'CSV', 'CUTQCUT', 'DECOMPRESS', 'DEFAULT', 'EXTRACT_JSONPATH', 'IPC', 'IS_IN', 'JSON', 'LAZY_REGEX', 'LIST_ANY_ALL', 'LIST_COUNT', 'LIST_SETS', 'LIST_TAKE', 'MERGE_SORTED', 'META', 'NIGHTLY', 'OBJECT', 'PARQUET', 'PCT_CHANGE', 'PERFORMANT', 'PIVOT', 'PROPAGATE_NANS', 'REPEAT_BY', 'RLE', 'SEARCH_SORTED', 'SERDE_JSON', 'SIGN', 'SQL', 'STREAMING', 'TIMEZONES', 'TOP_K', 'TRIGONOMETRY'], 'host': {'triple': 'x86_64-apple-darwin'}, 'target': {'arch': 'x86_64', 'os': 'macos', 'family': 'unix', 'env': '', 'triple': 'x86_64-apple-darwin', 'endianness': 'little', 'pointer-width': '64', 'profile': 'release'}, 'git': {'version': 'py-0.18.7', 'dirty': True, 'hash': '96b0e07e75a882eaed7be05e619307bc144aa5b4', 'head': None}, 'version': '0.18.7'}

@eddelbuettel
Copy link
Contributor

(FWIW I found SIMD is generally supported and eg package RcppSimdJson checks entirely at run-time which is most portable. We have run into issues with newer chipsets and AVX512 but only in cases where pre-made libraries are consumed. If you compile on the spot the toolchain generally knows.)

@eitsupi
Copy link
Collaborator Author

eitsupi commented Jul 15, 2023

(FWIW I found SIMD is generally supported and eg package RcppSimdJson checks entirely at run-time which is most portable. We have run into issues with newer chipsets and AVX512 but only in cases where pre-made libraries are consumed. If you compile on the spot the toolchain generally knows.)

Thanks for the information.
I think it's a Rust-specific issue for SIMD.

@eitsupi
Copy link
Collaborator Author

eitsupi commented Jul 15, 2023

I think you would be a better CRAN/polars maintainer then I. I happily swap roles and/or follow your lead in these matters.

Okay, I will submit 29a57de to CRAN. Let's see what happens with that. (BTW, I'm packing up my development PC for the move, so I won't be able to do any development work for the next few days.)

@eddelbuettel
Copy link
Contributor

And there it is:

image

Congratulations to all!

@eitsupi
Copy link
Collaborator Author

eitsupi commented Jul 18, 2023

Fantastic!
I really just submitted this package (used devtools::release(), took about 10 minutes)

@sorhawell @etiennebacher For now, do you think it would be a good idea to merge this as is for now and tag the merged commit?
(I actually submitted 8c2aec5, but I think it's better to squash this PR commit because it's messy and hard to see the history)

@eitsupi eitsupi marked this pull request as ready for review July 18, 2023 04:22
@sorhawell
Copy link
Collaborator

Wow. Long live the new maintainer !

@sorhawell
Copy link
Collaborator

sorhawell commented Jul 18, 2023

yeah with a tag and squash I guess this is ready to merge.

Copy link
Collaborator

@sorhawell sorhawell left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks good to me

@eddelbuettel
Copy link
Contributor

eddelbuettel commented Jul 18, 2023

CRAN giveth, CRAN taketh:

image

Over the failures on the r-devel machines I presume.

@sorhawell
Copy link
Collaborator

Very close.

@eitsupi
Copy link
Collaborator Author

eitsupi commented Jul 18, 2023

Sorry for the delay on my notification. I received two emails as follows.
I don't think the build time limitation issue (!) can be easily resolved, so I don't think we will be able to do a CRAN release for a while.

I think we need to merge this for now, release 0.7.0, and remove the installation instructions from CRAN from the README in a follow-up PR (sorry but I don't have the bandwidth to work on that for a while)


This was seen using 1500% CPU on the Fedora check system during
installation. As in

  • checking whether package ‘polars’ can be installed ... [6192s/525s] ERROR

which has also exceeded the allowed CPU time limit on that system.
That is a major violation of the CRAN policy, so the package will be
removed from CRAN.

Another is misrepresentation of authorship -- you have ignored the policy's
(’All components’ includes any downloaded at installation or during use.) and AFAICS not listed the authors and copyright holders of those you
download (and certainly not said who is responsible for those).


I finally found this. cargo build help says

Miscellaneous Options
    -j N, --jobs N
        Number of parallel jobs to run. May also be specified with the
        [build.jobs](http://build.jobs/) config value
        <https://doc.rust-lang.org/cargo/reference/config.html>.

Defaults to the number of logical CPUs.

So another thing for the 'Using Rust' document.

@sorhawell
Copy link
Collaborator

Another is misrepresentation of authorship -- you have ignored the policy's
(’All components’ includes any downloaded at installation or during use.) and AFAICS not listed the authors and copyright holders of those you
download (and certainly not said who is responsible for those).

Does that mean list authors of all crates?

@eitsupi
Copy link
Collaborator Author

eitsupi commented Jul 19, 2023

Does that mean list authors of all crates?

Yes, please check this post and the string2path package.
https://yutani.rbind.io/post/rust-and-cran-repository-policy/#doesnt-describe-the-authorship-and-copyright

@eitsupi
Copy link
Collaborator Author

eitsupi commented Jul 23, 2023

@sorhawell Could you push the tag and create the GitHub release?

@eitsupi
Copy link
Collaborator Author

eitsupi commented Jul 25, 2023

@sorhawell I think you made a mistake in tagging the wrong commit. I deleted the release and tag and pushed a new tag again.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants