Skip to content

Improve handling of duplicate divisions (i.e. empty partitions)#7214

Closed
ryan-williams wants to merge 2 commits intodask:mainfrom
celsiustx:divisions
Closed

Improve handling of duplicate divisions (i.e. empty partitions)#7214
ryan-williams wants to merge 2 commits intodask:mainfrom
celsiustx:divisions

Conversation

@ryan-williams
Copy link
Contributor

@ryan-williams ryan-williams commented Feb 14, 2021

  • Tests added / passed (existing tests updated)
  • Passes black dask / flake8 dask

This is a few early commits from #6661 (DDF iloc / partition_sizes); breaking it out for easier reviewing (and because it's mostly independent).

@ryan-williams
Copy link
Contributor Author

Seeing this error running make html in the Documentation GHA in #6661 and now here:

Exception occurred:
  File "/opt/hostedtoolcache/Python/3.7.9/x64/lib/python3.7/site-packages/sphinx/builders/html/__init__.py", line 1039, in <lambda>
    ctx['css_files'].sort(key=lambda js: js.priority)
AttributeError: 'str' object has no attribute 'priority'
The full traceback has been saved in /tmp/sphinx-err-z6ie2z11.log, if you want to report the issue to the developers.
Please also report this if it was a user error, so that a better error message can be provided next time.
A bug report can be filed in the tracker at <https://github.com/sphinx-doc/sphinx/issues>. Thanks!
make: *** [html] Error 2
Makefile:46: recipe for target 'html' failed

I don't see how it's related to my changes, but I'm also not seeing evidence of it in other builds or on the internet. I'll push an even more innocuous PR momentarily to get more data; maybe some unpinned pip dependency underneath sphinx has changed and broken things?

@ryan-williams ryan-williams force-pushed the divisions branch 2 times, most recently from bf9f891 to ce99177 Compare February 16, 2021 14:45
@ryan-williams ryan-williams changed the title WIP(iloc): improve handling of duplicate divisions (i.e. empty partitions) Improve handling of duplicate divisions (i.e. empty partitions) Feb 16, 2021
@ryan-williams
Copy link
Contributor Author

hm now seeing failures in test_base.py::test_visualize (and similar cases) on Windows, that I don't think are related to my changes. Example:

        # Start the process
        try:
>           hp, ht, pid, tid = _winapi.CreateProcess(executable, args,
                                     # no special security
                                     None, None,
                                     int(not close_fds),
                                     creationflags,
                                     env,
                                     cwd,
                                     startupinfo)
E                                    FileNotFoundError: [WinError 2] The system cannot find the file specified

C:\Miniconda3\envs\test-environment\lib\subprocess.py:1307: FileNotFoundError

@jsignell
Copy link
Member

Yeah that is a separate issue with windows. #7231

@jsignell
Copy link
Member

@ryan-williams could you provide some context for this change? Is there an issue that it fixes? Also it'd be good to know what the impact is on performance.

@jsignell jsignell added the needs review Needs review from a contributor. label Feb 28, 2022
Copy link
Member

@jsignell jsignell left a comment

Choose a reason for hiding this comment

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

Sorry this sat for so long @ryan-williams I think between on #8517 and #8806 the issue that this was trying to solve has likely been addressed. Thank you for working on this though!

@jsignell jsignell closed this Mar 16, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

dataframe io needs review Needs review from a contributor.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants