Remove more sdist files #259

ajfriend · 2022-06-04T22:18:58Z

I don't think there's any need for the following files to be included in the Python SDist:

src/h3lib/scripts
src/h3lib/examples
src/h3lib/src/apps

@isaacbrodsky, could you confirm that's true?

This will further reduce the SDist size (beyond the huge gains of #227), and may also decrease the build time (?).

However the build still fails, but I'm hoping that some small changes to the cmake files can avoid the need for these files?

codecov · 2022-06-04T22:21:01Z

Codecov Report

Merging #259 (302cae6) into dev_v4 (cfbd892) will not change coverage.
The diff coverage is n/a.

@@            Coverage Diff            @@
##            dev_v4      #259   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files           17        17           
  Lines          425       425           
=========================================
  Hits           425       425

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update cfbd892...302cae6. Read the comment docs.

ajfriend · 2022-06-04T22:57:54Z

I tried setting

set(BUILD_FUZZERS OFF CACHE BOOL "" FORCE)
set(ENABLE_LIBFUZZER OFF CACHE BOOL "" FORCE)
set(ENABLE_COVERAGE OFF CACHE BOOL "" FORCE)

but that didn't seem to help.

kylebarron · 2022-06-05T17:39:37Z

I think when I was stripping sdist size in #227, it didn't build out of the box if I removed those files. I figured the further gains were marginal so I didn't look more closely at how to make the build work after removing those.

isaacbrodsky · 2022-06-05T22:21:12Z

Was the error the same when BUILD_FUZZERS was OFF? Looking at CMakeLists.txt my suspicion is that the targets for executables are disabled, but other references to the source files remain, for example for autoformatting.

ajfriend · 2022-06-06T08:15:56Z

Was the error the same when BUILD_FUZZERS was OFF? Looking at CMakeLists.txt my suspicion is that the targets for executables are disabled, but other references to the source files remain, for example for autoformatting.

Getting different error output when I set BUILD_FUZZERS to OFF in the diff above.

isaacbrodsky · 2022-06-06T20:01:26Z

Yes, it indeed seems to be failing on the format step:

...
      -- Build files have been written to: /tmp/pip-req-build-88brxvzu/_skbuild/linux-x86_64-3.10/cmake-build
      [  2%] Formatting sources
      No such file or directory
      No such file or directory
...

I think to fix this we would need to either disable autoformatting (and thus reading/modifying those files) or would need to build up the sources list in a different way in uber/h3, where we only include files if their associated feature is enabled.

ajfriend · 2022-06-06T20:09:20Z

OK, cool. Let's just turn off autoformatting. I don't think there's a need for that at this stage. How would we configure that?

ajfriend · 2022-06-06T20:14:00Z

OK, cool. Let's just turn off autoformatting. I don't think there's a need for that at this stage. How would we configure that?

nvm. Just discovered the cmake -LA command. :)

isaacbrodsky · 2022-06-06T20:19:56Z

There's a reference at https://h3geo.org/docs/next/core-library/compilation-options

kylebarron · 2022-06-06T20:55:15Z

CMakeLists.txt

+# See all options with `cmake -LA` in an `h3/build` directory,
+# or at https://h3geo.org/docs/next/core-library/compilation-options/
+macro(turn_off option_name)
+ set(${option_name} OFF CACHE BOOL "" FORCE)
+endmacro()
+turn_off(BUILD_ALLOC_TESTS)
+turn_off(BUILD_BENCHMARKS)
+turn_off(BUILD_FILTERS)
+turn_off(BUILD_FUZZERS)
+turn_off(BUILD_GENERATORS)
+turn_off(BUILD_TESTING)
+turn_off(ENABLE_COVERAGE)
+turn_off(ENABLE_DOCS)
+turn_off(ENABLE_FORMAT)
+turn_off(ENABLE_LIBFUZZER)
+turn_off(ENABLE_LINTING)


Just curious, can you compare compilation times before and after this change? Does this make it a lot faster?

I spot checked some other compilation times. There's maybe a gain. I'll try some runs here for a clearer comparison.

Seems to be faster, based on the last two runs I looked at, but hard to tell how much is random variation.

Old config

New config

isaacbrodsky · 2022-06-06T20:58:05Z

CMakeLists.txt

+macro(turn_off option_name)
+ set(${option_name} OFF CACHE BOOL "" FORCE)
+endmacro()
+turn_off(BUILD_ALLOC_TESTS)


BUILD_ALLOC_TESTS should not have any effect when BUILD_TESTING is off. Harmless to disable it though.

I had a suspicion that that was true, but yeah, figured it couldn't hurt to just disable everything to make sure.

* WIP: Starting work on v4, The Sequel (#238) * cimport for h3fake2 doesn't seem to work * back to where we were * meh, close to what i was hoping for * h3fake2 in github actions * comment out current bindings that consume the h3c v3 library * don't need the coveragerc file at the moment; clear out c files in /tests * sdist needs h3fake2 * try CIBW_BEFORE_BUILD to get h3fake2 * need h3fake2 for cibuildwheel tests * update h3 C library * comment out most of h3lib.pxd * calling from v4 c lib works! (tests dont' work yet tho) * tests shall pass * starting to move over first few functions * little steps * util submodule fully moved over; some hackery with h3fake2 errors * getting even trickier with h3fake2 errors * few more simple functions * geo_to_h3 and h3_to_geo * grid_distance function * grid_disk * parents and kids * compact/uncompact * getNumCells * comment out sdist test temporarily * commenting out `make_sdist` seems to have turned off the other CI jobs * no, it was that we're pushing to the dev branch * keep the sdist job in there, but commented out * ajfriend/more v4 funcs (#241) * get_pentagon_indexes and get_res0_indexes * line * cell area functions * get_faces (#248) * initial pass * thoughts * the results of thinking * this commit is associated with the previous commits * nogil * i'm a bill * remove the typical nonsense * v4: local ij functions to v4 cython (#240) * move local ij functions to v4 implementation * revert some changes and add gridPathCells * swap implementation * revert gridPathCells * ring (#251) * Uncomment cython example, and exclude sdist from wheels test temporarilty (#252) * uncomment cython test * exclude make_sdist temporarily from the wheels tests * v4: polygonToCells (#254) * v4: geo boundary, set to multi polygon (#255) * v4: cell boundary * v4: h3_set_to_multi_polygon * v4: convert edge from h3fake2 (#253) * v4: convert edge from h3fake2 * on error not neighbors * except * * check for neighbor error * Fix sdist build (#239) * make sure `make_sdist` CI job still broken * reproduce locally * update h3lib * still seems broken * try sdist on mac and ubuntu * pyest! * debugging * ensurepath * Disable building docs * just build sdist once * colon Co-authored-by: Isaac Brodsky <[email protected]> * remove h3fake2 transitional library (#257) * Remove more sdist files (#259) * remove scripts/examples/apps * BUILD_FUZZERS OFF * ENABLE_FORMAT off * turn off all the things (and sort options) * mac attack * see if there's a build time difference * back to the future * New error system (#260) * starting to play around with new error system * move enum to h3lib.pxd * this structure is weird :( * error system gets its own file * go back to using H3ErrorCodes * extern the H3ErrorCodes * remove H3Index in favor of just H3int * " to ' * import ordering * something * exception hierarchy * formatting * errors list * remove H3ResolutionError * remove H3EdgeError * remove H3CellError * remove H3DistanceError * H3FailedError * domain error * H3UnrecognizedException * most of em done * remove H3ValueError * remove some checks * ideas ideas * document what the function raises? * thoughts * lint * one half idea for DRYing (kinda) * potentially make the E_SUCCESS path faster * typos * maple syrup * cimport H3ErrorCodes as ec * clean up * whitespace * clarify cases * trying new error idiom * thanks comment * notes * applied idiom a few times now * alternative idiom? * handling some more potential errors * idioms and ideas * clean * removing check * interesting allocation example * lint * just trying to beat 2.7 for sport right now * works? * bow to lint * using enum values directly! * now that's a hierarchy * remove raise_with_msg * ij funcs * lint * remove some checks * remove err * comment * cleaned up `ring`, and i think i fixed a memory leak bug * think i fixed another memory leak bug * another memory leak bug * memory leaks! * clean * removing some comments * meh * H3Exception shouldn't be appearing as a concrete object * renaming some base exceptions * clean up * don't have a test for this anyway * documented * puttering around * words * backing up this nonsense with an airtight argument * air toight * todos * more words * skepticism * rewording * editing * ordering * tests on the error codes * more error code tests * note * address some comments * we raise! * lint * comments * Squash #261: Use new H3MemoryManager system (#268) * Squash #261: Use new H3MemoryManager system Co-authored-by: AJ Friend <[email protected]> * add some error checks * comments * comments * clearer logical flow * arrays are different from memoryviews! * little cleaner * Revert "comments" This reverts commit 80c3f4d. * Revert "Revert "comments"" This reverts commit a9d6590. * Revert "comments" This reverts commit 24904bd. * comments * arrays are different from memoryviews! * try _copy_to_mv * i don't get enough opportunities to consider using the word "penultimate" * ugh, and there i go using it incorrectly... * hmmm. was this really all it was? * That's what it was! This reverts commit 7e72adb. * clarify branches * remove simple_mv * moving around * clean up * comments * comments * comments Co-authored-by: Isaac Brodsky <[email protected]> * remove: from h3 import h3 (#270) * is_valid_cell (#269) * Remove alias functions (#271) * remove alias function hex_range * note test_k_ring and test_hex_range are the same * remove test_hex_range * duplicate tests * remove test_hex_range2 * same test * remove test_hex_range_pentagon * remove alias: k_ring_distances * tests are the same! * same test * combine tests * remove alias: h3_is_res_class_iii * lint * Update most functions to v4.0 names (#272) * grid_distance * cell_to_parent * get_resolution * is_pentagon * is_res_class_III * get_pentagons * get_res0_cells * get_base_cell_number * compact and uncompact * get_faces * cell_to_boundary * cell_to_latlng * latlng_to_cell * cell_to_children * int_to_string and string_to_int * cell_to_center_child * moving things around * is_valid_directed_edge * grid_path_cells * moving around * cells_to_multi_polygon * are_neighbor_cells * cell_to_local_ij and local_ij_to_cell * get_icosahedron_faces * great_circle_distance * get_num_cells * note * remove hex_range_distances and hex_ranges (easily implementable by user) * grid_ring * grid_disk * moving around * origin_to_directed_edges * directed_edge_to_boundary * cells_to_directed_edge * directed_edge_to_cells * todo: directedEdgeToCells * get_directed_edge_origin * get_directed_edge_destination * grouping functions * more grouping * _binding to _b * todos * average_hexagon_area * average_edge_length * average_edge_length and edge_length * average_hexagon_edge_length * ordering * more ordering * Get the makefile working on Linux (#274) * bump core lib to 4.0 rc5 (#275) * bump core lib to 4.0 rc5 * more pythonic names for string<->int conversion functions * cython hex2int to str_to_int to match the python functions * better argument names for great_circle_distance * great_circle_distance comments * revert great_circle_distance tuple unpacking since it breaks python2. drop python 2 before release * polygon_to_cells (#276) * start converting tests over * converted main tests * linting * better null island test * test_compact_and_uncompact_cells * convert a few more tests * add h3.Polygon class * lint * migrating some tests * this shift_circular_list thing makes these tests too hard to understand... * remove shift_circular_list and convert a few more tests * last of the tests in test_h3.py converted * last of the tests converted * note for polygons_to_cells in the future * Try out a __repr__ for h3.Polygon * test_polygon_class.py * use `cells` instead of `hexes` throughout the library and tests * docstring for Polygon * docstrings for functions * use `res` convention for resolution parameters Co-authored-by: Isaac Brodsky <[email protected]> Co-authored-by: David Ellis <[email protected]>

remove scripts/examples/apps

d230846

BUILD_FUZZERS OFF

c9bb628

ENABLE_FORMAT off

1ac35eb

ajfriend added 2 commits June 6, 2022 13:20

turn off all the things (and sort options)

1617639

mac attack

4d8e59d

kylebarron approved these changes Jun 6, 2022

View reviewed changes

isaacbrodsky approved these changes Jun 6, 2022

View reviewed changes

ajfriend added 2 commits June 6, 2022 14:41

see if there's a build time difference

f9869aa

back to the future

302cae6

ajfriend merged commit 95c9aff into dev_v4 Jun 6, 2022

ajfriend deleted the ajfriend/remove_sdist_files branch June 6, 2022 23:37

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Remove more sdist files #259

Remove more sdist files #259

ajfriend commented Jun 4, 2022 •

edited

Loading

codecov bot commented Jun 4, 2022 •

edited

Loading

ajfriend commented Jun 4, 2022

kylebarron commented Jun 5, 2022

isaacbrodsky commented Jun 5, 2022

ajfriend commented Jun 6, 2022

isaacbrodsky commented Jun 6, 2022

ajfriend commented Jun 6, 2022

ajfriend commented Jun 6, 2022

isaacbrodsky commented Jun 6, 2022

kylebarron Jun 6, 2022

ajfriend Jun 6, 2022

ajfriend Jun 6, 2022

isaacbrodsky Jun 6, 2022

ajfriend Jun 6, 2022

Remove more sdist files #259

Remove more sdist files #259

Conversation

ajfriend commented Jun 4, 2022 • edited Loading

codecov bot commented Jun 4, 2022 • edited Loading

Codecov Report

ajfriend commented Jun 4, 2022

kylebarron commented Jun 5, 2022

isaacbrodsky commented Jun 5, 2022

ajfriend commented Jun 6, 2022

isaacbrodsky commented Jun 6, 2022

ajfriend commented Jun 6, 2022

ajfriend commented Jun 6, 2022

isaacbrodsky commented Jun 6, 2022

kylebarron Jun 6, 2022

Choose a reason for hiding this comment

ajfriend Jun 6, 2022

Choose a reason for hiding this comment

ajfriend Jun 6, 2022

Choose a reason for hiding this comment

isaacbrodsky Jun 6, 2022

Choose a reason for hiding this comment

ajfriend Jun 6, 2022

Choose a reason for hiding this comment

ajfriend commented Jun 4, 2022 •

edited

Loading

codecov bot commented Jun 4, 2022 •

edited

Loading