Skip to content

Navigation Menu

Sign in
Sign up

Subset dev - #14

Open
aplested wants to merge 14 commits into
master from
subset-dev
Open

Subset dev #14
aplested wants to merge 14 commits into
master from
subset-dev

Conversation

@aplested

@aplested aplested commented Mar 21, 2022

Copy link
Copy Markdown
Contributor

allow selection of a subset of traces for idealisation

aplested requested a review from eckuru March 21, 2022 08:49
Comment thread src/core/idealization.py
## self.idealize_series()
event_array = np.zeros((0, 5)).astype(object)
for episode in self.data.series:
if self.idealization(episode.n_episode) is not None:

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

This is the same as if self.idealization(episode.n_episode): so you don't have to use is not None at the end. Python treats None as well as empty list ([]) and empty string ('') as False in this context.

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.

You are right. Maybe I put an explanatory comment.

Comment thread src/core/recording.py
Comment on lines +142 to +143
if datakey is None:
datakey = self.current_datakey

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Another unimportant tip/comment: you can do this shorter like datakey = datakey or self.current_datakey.

Comment thread src/core/recording.py Outdated
Comment thread src/core/savedata.py
return_status = save_pickle(data=data, filepath=filepath)
>>>>>>> subsets
else:
print('Can only save as ".mat"!')

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Or as ".pkl" now :).

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Oh wait, actually no. The save_pickle function this is referring to is still commented out. Nevermind!

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.

I removed it again

Comment thread src/gui/analysis_widgets.py Outdated
selected_subsets,_ = self.main.ep_frame.subset_frame.subsets_check()
episodes = self.main.data.episodes_in_subsets(selected_subsets)
for ep in episodes:
print (f"Idealizing episode {ep.n_episode}")

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I didn't know this also worked.

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.

I don't know if this is something should react to or not!

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.

I added a catch now for the case that the list returned is empty because nothing was checked, with an error message to the terminal.

Comment thread src/gui/episode_frame.py Outdated
)
else:
self.parent.main.data.subsets[name][0].remove(index)
n = self.parent.ep_list.item(index).text()

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

You overwrite this right away... I'm generally a bit confused.

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.

Yes, that's true, very messy isn't it. I commented it out.

eckuru commented Mar 31, 2022

Copy link
Copy Markdown
Collaborator

I now looked at all the files, haven't yet run it myself. There are some minor changes to clean up that I had merged before, and merging this branch will reintroduce those, so it would be good to remove them. I think it should be possible to merge master locally and fix those and push, instead of doing manually. I usually can't guess right if git will think something is a conflict though.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@eckuru eckuru eckuru left review comments

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

AltStyle によって変換されたページ (->オリジナル) /