Skip to content

Conversation

@stevenhua0320
Copy link
Contributor

@zmx27 ready for review
-Remove unused modules
-change lambda def to standard def
-avoid bare except

Copy link
Collaborator

@zmx27 zmx27 left a comment

Choose a reason for hiding this comment

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

Minor comment

]
editor.refresh()
except:
except ImportError:
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why import error? I don't see any imports here...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Have changed to AttributeError, see the update

@stevenhua0320 stevenhua0320 changed the title delete unused module, change lambda def to def chore:delete unused module, change lambda def to def Sep 26, 2025
@stevenhua0320
Copy link
Contributor Author

@zmx27 Ready for another review

]
editor.refresh()
except:
except AttributeError:
Copy link
Collaborator

Choose a reason for hiding this comment

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

Do you think that this could also potentially result in a IndexError when the code can't find any TableEditorsBE objects? Could you look into this?

Copy link
Contributor Author

@stevenhua0320 stevenhua0320 Sep 27, 2025

Choose a reason for hiding this comment

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

Yes, you are right IndexError might also occur in the process. Have changed that in updated commit

Copy link
Collaborator

Choose a reason for hiding this comment

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

I think this PR looks good now

@stevenhua0320
Copy link
Contributor Author

@sbillinge Ready for review

Copy link
Contributor

@sbillinge sbillinge left a comment

Choose a reason for hiding this comment

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

pls see inline

@sbillinge sbillinge merged commit ab137e9 into diffpy:migration Sep 29, 2025
@sbillinge sbillinge deleted the migration-5 branch September 29, 2025 18:08
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.

3 participants