Add verification tests #428

troyraen · 2024-11-05T08:36:12Z

Change Description

My PR includes a link to the issue that I am addressing

Closes #118, closes #373, closes #374

Adds the following:

Verifier class
- Includes these tests: hats.io.validation.is_valid_catalog, schemas, file sets, row counts
- Outputs this file: verifier_results.csv
Unit tests
Test data
- Two malformed datasets: 'bad_schemas' and 'wrong_files_and_rows'
- Code to regenerate those datasets in case that is needed in the future.

Stil to-do:

Test a large catalog and determine whether tests may benefit from parallelization with dask.

Code Quality

I have read the Contribution Guide and LINCC Frameworks Code of Conduct
My code follows the code style of this project
My code builds (or compiles) cleanly without any errors or warnings
My code contains relevant comments and necessary documentation

New Feature Checklist

I have added or updated the docstrings associated with my feature using the NumPy docstring format
I have updated the tutorial to highlight my new feature (if appropriate)
I have added unit/End-to-End (E2E) test cases to cover my new feature
My change includes a breaking change
- My change includes backwards compatibility and deprecation warnings (if possible)

github-actions · 2024-11-05T08:42:24Z

Before [`57aaa9d`]	After [`0e37cc0`]	Ratio	Benchmark (Parameter)
failed	failed	n/a	benchmarks.BinningSuite.time_read_histogram

Click here to view all benchmarks.

troyraen · 2024-11-05T09:17:36Z

Unit tests are failing because of #427.

I need some guidance on the pre-commit failures. Most seem to be a result of path arguments being accepted as strings which are converted in the post_init, but the conversion isn't being recognized.

codecov · 2025-01-04T10:31:06Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 95.09%. Comparing base (c3fb952) to head (a7053a2).
Report is 4 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #428      +/-   ##
==========================================
+ Coverage   94.59%   95.09%   +0.50%     
==========================================
  Files          28       28              
  Lines        1627     1794     +167     
==========================================
+ Hits         1539     1706     +167     
  Misses         88       88

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

troyraen · 2025-01-04T11:45:39Z

@delucchi-cmu, this is finally ready for your review.

I tested it on the ZTF DR20 Lightcurves dataset (29176 partitions; 10TB) several times. It requires about 60GB RAM with a bit over half of that coming from hats.io.validation.is_valid_catalog. Total runtime is about 15 minutes, with this breakdown:

30 sec : Verifier.from_args (load datasets)
11.5 min : test_is_valid_catalog (just calls hats.io.validation.is_valid_catalog)
0.1 sec : test_file_sets
3 min : test_num_rows
2 sec : test_schemas

It does not currently use Dask. If the times above are reasonably representative for other large catalogs (which they may or may not be; idk) then I don't think it needs it. Let me know what you think there.

src/hats_import/verification/arguments.py

src/hats_import/verification/run_verification.py

delucchi-cmu · 2025-01-06T18:43:33Z

src/hats_import/verification/run_verification.py

+        )
+
+        # check user-supplied total, if provided
+        if self.args.truth_total_rows is not None:


There is also a catalog.catalog_info.total_rows attribute that should match the user provided value. And that should match the _metadata-derived value.

delucchi-cmu · 2025-01-06T18:48:06Z

tests/hats_import/verification/generate_malformed_catalogs.py

+INPUT_CATALOG_DIR = DATA_DIR / "small_sky_object_catalog"
+
+
+def run(input_catalog_dir: Path = INPUT_CATALOG_DIR, data_dir: Path = DATA_DIR) -> None:


In general, test data is generated via this notebook, so that if we make other format changes, we can quickly re-generate all unit test data in one go.

https://github.com/astronomy-commons/hats-import/blob/main/tests/data/generate_data.ipynb

So there are a few options for this PR (in no particular order):

1 - incorporate into the existing notebook
2 - create a second notebook in that same directory so folks can easily find it when re-generating unit test data
3 - skip for this PR, and maybe move it in the future.

This kind of thing is generally the trickiest part of writing verification (making data that's bad enough to trip the checks), so I really appreciate that you took the time to do it, and make it reproducible!!

Co-authored-by: Melissa DeLucchi <[email protected]>

…parison is fraught).

hombit · 2025-01-08T15:02:05Z

tests/hats_import/verification/test_run_verification.py

-        str(COMMON_DIR / "Npix=11.missing_file.parquet"),
+    expected_bad_file_names = {"Npix=11.extra_file.parquet", "Npix=11.missing_file.parquet"}
+    actual_bad_file_names = {
+        file_name.replace("\\", "/").split("/")[-1] for file_name in verifier.results_df.bad_files.squeeze()


I think you can use os.path.basename here

troyraen mentioned this pull request Nov 5, 2024

Add verification tests #379

Closed

14 tasks

troyraen self-assigned this Nov 20, 2024

troyraen mentioned this pull request Dec 14, 2024

TypeError when converting ZTF Lightcurves from hipscat -> hats #458

Closed

3 tasks

troyraen force-pushed the raen/add/verification branch from b0b464a to 1950217 Compare January 2, 2025 18:03

troyraen added 4 commits January 4, 2025 00:29

update verification

d8b3417

update conftest.py

b543fe0

add test data bad_schemas_dir, wrong_files_and_rows_dir

42a87f7

update tests

7b2fb2f

troyraen force-pushed the raen/add/verification branch from 1950217 to 7b2fb2f Compare January 4, 2025 08:30

troyraen added 5 commits January 4, 2025 02:24

Cleanup: fix typing for mypy, use hats file pointers, etc.

ea6940e

Remove special handling of pandas metadata

0e13153

Add code to generate test data

0a5970a

Update test data

a0bbd66

Complete unit test coverage

4a3445a

Fix unit tests for Windows

6f815da

troyraen requested a review from delucchi-cmu January 4, 2025 11:45

delucchi-cmu reviewed Jan 6, 2025

View reviewed changes

Apply suggestions from code review

575d06e

Co-authored-by: Melissa DeLucchi <[email protected]>

delucchi-cmu mentioned this pull request Jan 7, 2025

Add a test to exercise the verification pipeline on a cloud catalog. astronomy-commons/hats-cloudtests#54

Open

delucchi-cmu added 2 commits January 7, 2025 15:55

Support non-local file systems

11c14ca

Fix for windows (pathlib internally uses "/" for paths, so string com…

0439016

…parison is fraught).

hombit reviewed Jan 8, 2025

View reviewed changes

Another fix for windows path string comparison

a7053a2

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add verification tests #428

Add verification tests #428

troyraen commented Nov 5, 2024 •

edited

Loading

github-actions bot commented Nov 5, 2024

troyraen commented Nov 5, 2024

codecov bot commented Jan 4, 2025 •

edited

Loading

troyraen commented Jan 4, 2025

delucchi-cmu Jan 6, 2025

delucchi-cmu Jan 6, 2025

delucchi-cmu Jan 6, 2025

hombit Jan 8, 2025

		INPUT_CATALOG_DIR = DATA_DIR / "small_sky_object_catalog"


		def run(input_catalog_dir: Path = INPUT_CATALOG_DIR, data_dir: Path = DATA_DIR) -> None:

Add verification tests #428

Are you sure you want to change the base?

Add verification tests #428

Conversation

troyraen commented Nov 5, 2024 • edited Loading

Change Description

Code Quality

New Feature Checklist

github-actions bot commented Nov 5, 2024

troyraen commented Nov 5, 2024

codecov bot commented Jan 4, 2025 • edited Loading

Codecov Report

troyraen commented Jan 4, 2025

delucchi-cmu Jan 6, 2025

Choose a reason for hiding this comment

delucchi-cmu Jan 6, 2025

Choose a reason for hiding this comment

delucchi-cmu Jan 6, 2025

Choose a reason for hiding this comment

hombit Jan 8, 2025

Choose a reason for hiding this comment

troyraen commented Nov 5, 2024 •

edited

Loading

codecov bot commented Jan 4, 2025 •

edited

Loading