- 
                Notifications
    You must be signed in to change notification settings 
- Fork 51
Breaking #946
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
base: main
Are you sure you want to change the base?
Conversation
… for `AbstractDimStack` (#903) * add combine method * test groupby and similar * docs entry
| Codecov Report❌ Patch coverage is  
 Additional details and impacted files@@            Coverage Diff             @@
##             main     #946      +/-   ##
==========================================
- Coverage   86.90%   82.66%   -4.24%     
==========================================
  Files          55       57       +2     
  Lines        5338     5631     +293     
==========================================
+ Hits         4639     4655      +16     
- Misses        699      976     +277     ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
 | 
* add preservedims keyword to DimTable * add tests * Apply suggestions from code review Co-authored-by: Anshul Singhvi <[email protected]> * tests, and fix DimSlices * better table docs * cleanup * test * indexing overhaul * fix similar and broadcast for basicdimarray * bugfix rebuildsliced * more indexing cleanup * cleanup similar and gubfix indexing * bugfixes * uncomment * fix doctests * just dont doctest unreproducable failures, for now * combine new Tables integrations * bugfix and cleanup show * bugfix and more tests for preservedims and mergedims --------- Co-authored-by: Anshul Singhvi <[email protected]>
* iterate values where no layer is missing * add tests * add skipmissing to reference
* Table Materializer Methods * Made col Optional for DimArray * Apply suggestions from code review Co-authored-by: Rafael Schouten <[email protected]> * Handle coordinates with different loci * replaced At() with Contains() in _coords_to_ords * Added optional selectors and public methods for table materializer * Updated table constructors for DimArray and DimStack * Updated DimArray and DimStack docs to include table materializer methods * Table materializer test cases * export table materializer methods * Added Random to tables.jl test cases * Update src/array/array.jl Co-authored-by: Rafael Schouten <[email protected]> * Update src/table_ops.jl Co-authored-by: Rafael Schouten <[email protected]> * Removed exports * Update src/table_ops.jl Co-authored-by: Rafael Schouten <[email protected]> * Update src/table_ops.jl Co-authored-by: Rafael Schouten <[email protected]> * Update src/table_ops.jl Co-authored-by: Rafael Schouten <[email protected]> * Update src/table_ops.jl Co-authored-by: Rafael Schouten <[email protected]> * Update src/table_ops.jl Co-authored-by: Rafael Schouten <[email protected]> * Replaced selector type with instance. * Table materializer can now infer dimensions from the coordinates. * Update src/stack/stack.jl Co-authored-by: Rafael Schouten <[email protected]> * Update src/table_ops.jl Co-authored-by: Rafael Schouten <[email protected]> * Update src/table_ops.jl Co-authored-by: Rafael Schouten <[email protected]> * Update src/table_ops.jl Co-authored-by: Rafael Schouten <[email protected]> * Update src/table_ops.jl Co-authored-by: Rafael Schouten <[email protected]> * Update src/table_ops.jl Co-authored-by: Rafael Schouten <[email protected]> * Update src/table_ops.jl Co-authored-by: Rafael Schouten <[email protected]> * Update src/array/array.jl Co-authored-by: Rafael Schouten <[email protected]> * Update src/table_ops.jl Co-authored-by: Rafael Schouten <[email protected]> * Added support for guessing the dimension ordering and span for Dates and DateTimes * Replaced LinRange with StepRangeLen in _build_dim * Added Tables.istable check to DimArray constructor * Update src/array/array.jl * merge materialize2 * fix scuffed merge * filter instead of indexing in test for clarity * fix DimSlices doc * fix ambiguities * bugfixes * do checks and call Tables.columns before constructing stack from table * test dimensions are automatically detected when constructing dimstack * comments not docstrings for internals * check for columnaccess if dims are passed * add type argument to dimarray_from_table * allow passing name to DimStack * add a section to the documentation * use Tables.columnnames instead of keys * make DimArray work with all tables that are abstractarrays * do not treat dimvectors as tables * simplify get_column --------- Co-authored-by: Rafael Schouten <[email protected]> Co-authored-by: Tiem van der Deure <[email protected]>
| @tiemvanderdeure could you possibly resolve the broadcast.jl conflicts locally and PR them? (merge main into broken and just keep the changes to broadcast.jl) There is now the BasicDimArray broadcast style, and your changes clash, and I'm not totally accross the new broadcast changes to know what to keep. I can resolve the other conflicts afterwards | 
| Ok CHANGELOG.jl added, closing #983 After this I want to merge to main and bump a new breaking version, if there are any reviews or complaints about that get them in in the next 24 hours! | 
* add _similar dispatch for abstractdimarray * update tests * Update src/array/array.jl Co-authored-by: Rafael Schouten <[email protected]> * Update src/array/array.jl Co-authored-by: Rafael Schouten <[email protected]> --------- Co-authored-by: Rafael Schouten <[email protected]>
* standardise interface methods and remove index * update Changelog * cleanup * move const * cleanup * remove index from test * dont export index * last index * tweaks * more tweaks * fix tests --------- Co-authored-by: Raf Schouten <[email protected]>
* Forward name keyword in groupby * Add test for setting groupby name explicitly * Update src/groupby.jl Co-authored-by: Rafael Schouten <[email protected]> * Update test/groupby.jl Co-authored-by: Rafael Schouten <[email protected]> * Add Changelog entry * Mention name keyword in docstring --------- Co-authored-by: Rafael Schouten <[email protected]>
| Do you want to also merge the other breaking branches as well or what is your timeline for the breaking release? | 
| I'm not sure they will all make it, or how long it's worth waiting | 
* Remove rtol from At selector * Remove explicit rtol from test * Remove unused type parameter * fix At constructors --------- Co-authored-by: Rafael Schouten <[email protected]>
| I just ran some tests locally on this branch and a bunch of them fail because 3-argument  I want to give the instantiate thing another shot. I just tested with Rasters and it just needs some minor changes to work. I figured I might as well make it on a branch from this branch, so we don't run into the merge conflicts again (and I suspect I might be able to get rid of the  | 
| Do you have an idea why code coverage drops so significantly? It seems to be a lot of indirect changes but the test changes seem to be mainly removing index. I am wondering whether we are not hitting certain dispatch routes not anymore. | 
…tions (#1113) * do 0.6, 0.7.2 broke for us (#1099) * Fix tests on julia 1.12 (#1110) * use isequal instead of === to compare NaN * drop all and broadcast * specify DimensionalData.Dimensions to make reference unique in docs * drop convert method for name to abstractstring * remove `merge` method for dimstack with iterators of pairs * add to changelog --------- Co-authored-by: Lazaro Alonso <[email protected]>
| @codecov-ai-reviewer test | 
| On it! Codecov is generating unit tests for this PR. | 
| Sentry has determined that unit tests are not necessary for this PR. | 
| lol, codecov is lazy on this one | 
| Should we make a plan for which PR's we still want to include and when we want to release this? Just so it doesn't drag on forever | 
| @tiemvanderdeure isn't it just your broadcast PR? I'm not waiting for anything else | 
| There are a few open PRs that are marked breaking like #847 #926 #991 and possibly including refdims in tables by default #1119. Are we including any of those as well or is that for the next breaking release? I think #1118 is already safe to merge - but I can make a Rasters branch for DD 0.30 later today and run tests to double check. | 
| Yeah please test in Rasters first. Those branches are all breaking but none of them are ready, so they can wait. | 
* implement `instantiate` - get rid of BasicDimensionalStyle * fix setindex! for opaquearray to make some error messages clearer * fix materialize!
all finialised breaking changes for the next breaking version