search

QEDjl-project / QEDcore.jl

[WIP] Core types and functions for QED.jl
https://qedjl-project.github.io/QEDcore.jl/stable/
MIT License
0 stars 3 forks source link

Revoke namespace change #29

Closed AntonReinhard closed 3 months ago

AntonReinhard commented 3 months ago

This PR revokes the namespace changes done before QEDbase.jl 0.2 was released.

Currently based on https://github.com/QEDjl-project/QEDcore.jl/pull/25, I will rebase once that is merged.

AntonReinhard commented 3 months ago

I've also deleted the ProcessSetup tests since fixing them started taking too long and they are deprecated anyways.

AntonReinhard commented 3 months ago

General question since we will now often implement interfaces defined somewhere else (i.e. in QEDbase): I think for clarity it would be a good idea to always use function QEDbase.interface_function() ... end directly, because it makes it clear right there that it's implementing an interface. The other option is having an import QEDbase: interface_function at the start of the file. This, in my opinion, kind of hides the fact that when we have function interface_function() ... end later, it is implementing an interface from QEDbase.

I've done it using QEDbase.interface_function mostly, but not everywhere. If we want to make that a guideline I will go through this pr again and do it in the remaining places.

Thoughts?

szabo137 commented 3 months ago

General question since we will now often implement interfaces defined somewhere else (i.e. in QEDbase): I think for clarity it would be a good idea to always use function QEDbase.interface_function() ... end directly, because it makes it clear right there that it's implementing an interface. The other option is having an import QEDbase: interface_function at the start of the file. This, in my opinion, kind of hides the fact that when we have function interface_function() ... end later, it is implementing an interface from QEDbase.

I've done it using QEDbase.interface_function mostly, but not everywhere. If we want to make that a guideline I will go through this pr again and do it in the remaining places.

Thoughts?

As I mentioned somewhere else, I think QEDbase.interface_function is favorable, because of the reasons you already mentioned. Therefore, I agree with making this the guideline.

szabo137 commented 3 months ago

@AntonReinhard is there a reason, that this is still a draft?

AntonReinhard commented 3 months ago

@AntonReinhard is there a reason, that this is still a draft?

Yes, as I mentioned I will go through again and remove the import QEDbase: ... occurrences. Then I'll mark it ready.