Skip to content

Adding precision to visualize args#8710

Merged
jcrist merged 4 commits intodask:mainfrom
Dranaxel:patch-1
Feb 15, 2022
Merged

Adding precision to visualize args#8710
jcrist merged 4 commits intodask:mainfrom
Dranaxel:patch-1

Conversation

@Dranaxel
Copy link
Copy Markdown
Contributor

@GPUtester
Copy link
Copy Markdown
Collaborator

Can one of the admins verify this patch?

Copy link
Copy Markdown
Member

@pavithraes pavithraes left a comment

Choose a reason for hiding this comment

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

@Dranaxel Thanks for this PR!

dask/base.py Outdated
----------
args : object
Any number of objects. If it is a dask object, its associated graph
Any number of objects. If it is a dask collection - i.e Dataframe, Array,
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

There is a trailing whitespace on this line, that's why the CI linter is failing.

Also, maybe we can say "e.g.," or "like" instead of "i.e" because Dask also allows you to create custom collections

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I generally prefer to avoid Latin abbreviations in documentation, such as i.e. or e.g.. If you aren't an academic, it can be hard to know what they are. "That is" is a good replacement for i.e. and "For example" is a good replacement for e.g..

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

@bryanwweber Thanks for this note, it's a very good point. I'll also keep it in mind moving forward!

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Sure thing @pavithraes ☺️ it's one of those things that you don't notice until someone says something and then you are like, how did I not see this before? At least, that's how it was for me 🤪

Fixing trailing space
Rewriting to avoid unclear expressions
dask/base.py Outdated
----------
args : object
Any number of objects. If it is a dask object, its associated graph
Any number of objects. If it is a dask collection - That is Dataframe, Array,
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I think @pavithraes was suggesting that this should be "For example, Dataframe, Array,"

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

As you want but I was applying what you said to me earlier:

"That is" is a good replacement for i.e. and "For example" is a good replacement for e.g...

But if you confirm you prefer for example I will go for it

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Yes, that's what I said (but I meant it as a more general comment). I didn't intend to disagree with @pavithraes who suggested "e.g." or "like", and I intended that you should take her suggestion of "e.g." and replace that with "For example"

@jcrist jcrist merged commit 4f44ab1 into dask:main Feb 15, 2022
@jcrist
Copy link
Copy Markdown
Member

jcrist commented Feb 15, 2022

Thanks @Dranaxel!

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.

Improve docstring for dask.visualize

5 participants