Skip to content

Avoid replacing so many characters with _ in _parse_raw_sam_df() #768

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
Peque opened this issue Aug 23, 2019 · 8 comments
Open

Avoid replacing so many characters with _ in _parse_raw_sam_df() #768

Peque opened this issue Aug 23, 2019 · 8 comments

Comments

@Peque
Copy link
Contributor

Peque commented Aug 23, 2019

As suggested by @wholmgren.

I too think it would be more convenient. For example, it would allow for easier distinction between manufacturer and module number.

@Peque
Copy link
Contributor Author

Peque commented Sep 22, 2019

@wholmgren @cwhanse Since we already broke the API by changing the module and inverter databases (i.e.: module/inverter names need to be updated to work with 0.7), would you be okay to introduce this change as well?

Asking before spending time preparing the PR. 😊

@Peque
Copy link
Contributor Author

Peque commented Sep 22, 2019

Just adding another vote in favor of this change:

@adriesse

I've never use the tab completion and find the database more convenient in its non-transposed form.

@cwhanse
Copy link
Member

cwhanse commented Sep 26, 2019

I agree with making changes to the module dataframe keys. I think our goal is to remain as consistent as possible with the source. We can propose changes to the source, if that's the best course. The SAM developers agree with the overall goal to have one name for a module that is common among both codes.

@Peque
Copy link
Contributor Author

Peque commented Sep 26, 2019

@cwhanse @wholmgren I would propose to have a separator between the manufacturer name and the model name, either a , (separate columns in the CSV) or some other character. It used to be the case (a : was used), but it seems not anymore. 😕

I opened this issue, in case you want to upvote/comment: NREL/SAM#227

I may wait for that to be resolved before implementing this change (feeling lazy, in case I have to change the names 2 times... 😂)

@cwhanse
Copy link
Member

cwhanse commented Nov 5, 2019

@Peque looking for closure on v0.7 release. Would you prefer to defer work in pvlib on this issue, pending any changes in SAM?

@Peque
Copy link
Contributor Author

Peque commented Nov 6, 2019

@cwhanse Thanks for the ping.

I would prefer to defer work pending changes in SAM, if you do not mind.

@cwhanse
Copy link
Member

cwhanse commented Nov 6, 2019

@Peque don't mind at all.

@Peque
Copy link
Contributor Author

Peque commented Jun 13, 2020

Maybe related (creating a separate quick-to-load Python package for parts): https://groups.google.com/forum/#!topic/pvlib-python/Sqe84M2C6RY

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

No branches or pull requests

2 participants