Skip to content

Conversation

@uribgp
Copy link
Contributor

@uribgp uribgp commented Dec 23, 2020

Fixes #1496

Description: Breaking up large pull

Check list:

  • New tests are added (if a new feature is added)
  • New doc strings: description and/or example code are in RST format
  • Documentation is updated (if required)

uribgp and others added 30 commits December 15, 2020 17:51
This reverts commit 003956e.
@uribgp
Copy link
Contributor Author

uribgp commented Dec 23, 2020

There is one file remaining, unfortunately I'm a bit stuck on it.

        output_message = """
 ----------------------------------------------------
| Time profiling stats (in seconds):                 |
 ----------------------------------------------------
total  |  min/index  |  max/index  |  mean  |  std

Processing function:
{processing_stats}

Dataflow:
{dataflow_stats}

Event handlers:
{total_time:.5f}

- Events.STARTED: {STARTED_names}
{STARTED}

- Events.EPOCH_STARTED: {EPOCH_STARTED_names}
{EPOCH_STARTED}

- Events.ITERATION_STARTED: {ITERATION_STARTED_names}
{ITERATION_STARTED}

- Events.ITERATION_COMPLETED: {ITERATION_COMPLETED_names}
{ITERATION_COMPLETED}

- Events.EPOCH_COMPLETED: {EPOCH_COMPLETED_names}
{EPOCH_COMPLETED}

- Events.COMPLETED: {COMPLETED_names}
{COMPLETED}
""".format(
            processing_stats=odict_to_str(results["processing_stats"]),
            dataflow_stats=odict_to_str(results["dataflow_stats"]),
            **others,
        )

I'm confused on how to break up the **others.

There are several lines which are similar to:
row[3] = "{}/{}".format(*row[3]) # type: ignore[misc]
I tried to do it
f"{row[3][0]}/{row[3][1]}"
but it was returning errors.

@vfdev-5
Copy link
Collaborator

vfdev-5 commented Dec 23, 2020

@uribgp let's keep format here. There is nothing wrong to use it there.

Btw, please, with formatting as it is failing right now.

@uribgp
Copy link
Contributor Author

uribgp commented Dec 23, 2020

OK. In that case this should be the final pull. I'll check why these are failing.

@vfdev-5
Copy link
Collaborator

vfdev-5 commented Dec 23, 2020

Please, set Fixes #1496 in the description then and once this PR is merged it will autoclose the issue. And we can also close the large PR.

Copy link
Collaborator

@vfdev-5 vfdev-5 left a comment

Choose a reason for hiding this comment

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

Looks good ! Thanks a lot for such a huge work, @uribgp !
I was just wondering if you could rerun jupyter notebooks after the modification ?

@vfdev-5 vfdev-5 merged commit 3061d5e into pytorch:master Dec 24, 2020
@uribgp
Copy link
Contributor Author

uribgp commented Dec 24, 2020

I didn't try to run all of the cells, just the ones I edited. It seemed to be working though.

Thanks for all the help with this!

@vfdev-5
Copy link
Collaborator

vfdev-5 commented Dec 24, 2020

I didn't try to run all of the cells, just the ones I edited. It seemed to be working though.

Sounds good !

Feel free to check out later other "help wanted" issues if you'd like to help with this project 👍

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.

Replace string .format with f-string

2 participants